Skip to content

Split Buddy extended backends to seperate files#13

Merged
trdthg merged 1 commit into
RuyiAI-Stack:riscvfrom
trdthg:riscv
Jun 27, 2026
Merged

Split Buddy extended backends to seperate files#13
trdthg merged 1 commit into
RuyiAI-Stack:riscvfrom
trdthg:riscv

Conversation

@trdthg

@trdthg trdthg commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

@Yranger @wujing215 Would you like to review this PR? I have refactored all our custom backend support to the standard RISC-V extension style.
You can now enable the respective backend support in buddy-llc by passing the -mattr=+x[vendor_name][extension_name] flag.

Also there are two technical details regarding the extension and register naming that I'd like to share or get your feedback on:

  • During the previous migration, I discovered that the register names M0-7 were already occupied by upstream LLVM (used as mask registers). To avoid conflicts, I have renamed them to AMEM0-7.
  • The BOSC AME extension has a 64-bit instruction encoding variant, and I'm a bit com about the standard way to represent this in LLVM. Can/Should both the 32-bit and 64-bit encodings be enabled at the same time? For now, I separated them into two distinct names: boscame (for 32-bit) and boscame64 (for 64-bit).

@Yranger

Yranger commented Jun 10, 2026

Copy link
Copy Markdown

For register names M0-7, they can be renamed to AMEM0-7, which can be achieved by simply changing the register names I call; however, I noticed that in the modified files, the RISCVInstrInfoBuddyBOSCExt.td and RISCVInstrInfoBuddyExt.td files are split into XT and BOSC parts respectively, but the BOSC part I designed and implemented is for the 32-bit instruction format, whereas I found that the RISCVInstrInfoXBOSCAME.td file is implemented for 64-bit instructions.

@trdthg

trdthg commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator Author

Ah, I think I've mixed up some things. The 64-bit AME instruction was introduced by @wujing215. Could you please help explain it to me? Where is the spec? Are they defined by XTheadAME?

@Yranger

Yranger commented Jun 10, 2026

Copy link
Copy Markdown

Alright, the developers you mentioned were focused on the design and implementation of IME, as well as a simple initial version of AME (which uses a 64-bit instruction format); however, my work involves the development of 32-bit Xuantie XTAME and the development and implementation of 32-bit BOSCAME for Kaixin Institute. The backend instruction format definition documents I am involved with are the XTAME 32-bit section in the latter part of the file llvm/lib/Target/RISCV/RISCVInstrInfoBuddyExt.td and the 32-bit BOSCAME development section in the file llvm/lib/Target/RISCV/RISCVInstrInfoBuddyBOSCExt.td.

@trdthg

trdthg commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator Author

So what's the current status of the simple initial version of AME (which uses a 64-bit instruction format)? Which version of spec does it belong to? Is it still necessary for us to support it?

@Yranger

Yranger commented Jun 10, 2026

Copy link
Copy Markdown

What we are mainly doing now is supporting BOSCAME (32-bit instruction format) of Kaixin Institute, which can be directly compiled using llc in LLVM, as the compiler used there is also based on LLVM's llc; however, the XTAME extension specific to Xuantie requires changes, adding the corresponding attributes for the Xuantie extension so that the header attributes in the compiled assembly file contain the corresponding Xuantie extension name.

@trdthg

trdthg commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator Author

Okay, I've removed the 64-bit part. See if that works? (I can also help you test if there is a publicly available testing document)

@Yranger

Yranger commented Jun 10, 2026

Copy link
Copy Markdown

Alright, I have already understood the modifications to the relevant documents. I am currently working on improving instruction coverage for Xuantie and the Open Core Institute. So I will modify my current files according to what you suggested, and then submit the PR afterwards.

@WuXintong123
WuXintong123 force-pushed the riscv branch 8 times, most recently from bef6bf7 to 7e401a7 Compare June 20, 2026 03:11
@WuXintong123
WuXintong123 force-pushed the riscv branch 3 times, most recently from ab6c6a2 to e17fd0c Compare June 23, 2026 00:04
WuXintong123 pushed a commit that referenced this pull request Jun 23, 2026
On targets where `size_t` is narrower than 64 bits (e.g. `i686`), CIR
codegen for `sizeof`/`alignof`/`__builtin_vectorelements` crashes with a
type/value bitwidth mismatch.

The result of these expressions is `size_t`, but the emitted integer
constant was built with a hardcoded 64-bit type. `EvaluateKnownConstInt`
returns an `APSInt` with the width of the AST result type (32 bits on
this target), so it no longer matches the `IntAttr`'s type and trips the
`IntAttr` verifier.

  ### How to Reproduce
  ```c++
  using size_t = decltype(sizeof(int));
  size_t size_of_int() { return sizeof(int); }

  clang -cc1 -std=c++20 -triple i686-unknown-linux-gnu -fclangir \
    -emit-cir test.cpp -o test.cir

  error: type and value bitwidth mismatch: 64 != 32
  clang: mlir/include/mlir/IR/StorageUniquerSupport.h:180:
    ... Assertion `succeeded(ConcreteT::verifyInvariants(...))' failed.
   #11 cir::IntAttr::get(mlir::Type, llvm::APInt const&)
   #12 ScalarExprEmitter::VisitUnaryExprOrTypeTraitExpr(...)
   #13 CIRGenFunction::emitScalarExpr(...)
```
### Fix
Form the IntAttr using the converted AST result type (convertType(e->getType())) instead of a hardcoded 64-bit type, matching classic codegen. The same path covers the fixed-vector __builtin_vectorelements case.
@WuXintong123
WuXintong123 force-pushed the riscv branch 2 times, most recently from 7027b71 to fb4a67e Compare June 25, 2026 03:38
@zhanghb97

Copy link
Copy Markdown

@trdthg What is the status of this PR? Is it ready for review, or is it still a work in progress?

@trdthg trdthg closed this Jun 26, 2026
@trdthg trdthg reopened this Jun 26, 2026
@trdthg
trdthg merged commit 3eb50c8 into RuyiAI-Stack:riscv Jun 27, 2026
10 of 13 checks passed
WuXintong123 pushed a commit that referenced this pull request Jul 21, 2026
…builtin (llvm#210524)

## Summary
Fix an assertion failure when Clang classifies a built-in call with
type-dependent arguments before template instantiation, e.g., while
deducing an `auto` non-type template parameter:

  For example:
  ```cpp
  template <auto> struct S {};

  template <typename T>
  using Alias = S<__builtin_constant_p(T::x)>;
```
```
Assertion failed: (isa<T>(CanonicalType)), function castAs, file
TypeBase.h, line 9349.
 #9  clang::Type::castAs<clang::FunctionType>() const
 #10 clang::CallExpr::getCallReturnType(clang::ASTContext const&) const
 #11 ClassifyInternal(clang::ASTContext&, clang::Expr const*)
#12 clang::Expr::ClassifyImpl(clang::ASTContext&,
clang::SourceLocation*) const
 #13 clang::Sema::DeduceAutoType(...)
 #14 clang::Sema::CheckTemplateArgument(...)
```

## Cause
Built-in references initially have the BuiltinFn placeholder type. For a non-dependent call, `BuildResolvedCallExpr()` applies `CK_BuiltinFnToFnPtr`, converting the callee to its function-pointer type. 

When a call has type-dependent arguments, `BuildCallExpr()` postpones semantic analysis until instantiation. The callee, therefore, retains its BuiltinFn placeholder type. Deducing the auto non-type template parameter classifies the dependent call through `Expr::ClassifyImpl()`. This calls `CallExpr::getCallReturnType()`, which previously did not handle BuiltinFn and attempted to cast the placeholder to FunctionType, triggering an assertion.

The issue also affects ordinary builtins such as `__builtin_ffs` and is not specific to` __builtin_constant_p`.

 ## Fix
Handle `BuiltinFn` alongside the dependent and `Overload` callee cases in `CallExpr::getCallReturnType()` and return `DependentTy`. This matches the type `BuildCallExpr()` gives the `CallExpr` itself, so `getCallReturnType()` and `getType()` now agree on such calls.

Built-in resolution and built-in-specific type checking still occur as usual during template instantiation.

Added coverage for dependent calls and successful instantiation of `__builtin_constant_p` and `__builtin_ffs`,  a non-dependent control case, and getCallReturnType() for a BuiltinFn callee.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants