[clang][RISCV] support BITINT with mixed-type #156592
Conversation
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this comment.
Pull Request Overview
This PR implements mixed-type support for BITINT in RISC-V by making type extension based on the variable type rather than using sign extension for all cases, aligning with the RISC-V ELF PSABI specification.
Key changes:
- Updates RISC-V ABI handling to use appropriate extension (zero/sign) based on BitInt signedness
- Adds comprehensive test coverage for signed, unsigned, and default BitInt types across different bit widths
- Modifies existing test expectations to reflect the new signext behavior for signed BitInt parameters
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| clang/test/CodeGen/ext-int-cc.c | Updates test expectations to include signext attribute for RISC-V BitInt parameters |
| clang/test/CodeGen/RISCV/bitint.c | Adds comprehensive test cases for BitInt operations with different signedness and bit widths |
| clang/lib/CodeGen/Targets/RISCV.cpp | Implements the core ABI changes to handle BitInt extension based on type signedness |
| // FIXME: Maybe we should treat 32 as a special case and wait for | ||
| // the SPEC to decide. |
There was a problem hiding this comment.
The FIXME comment indicates uncertainty about handling 32-bit BitInt types and references waiting for a specification decision. This suggests the implementation may be incomplete or temporary for this specific case.
| // FIXME: Maybe we should treat 32 as a special case and wait for | |
| // the SPEC to decide. | |
| // FIXME: The handling of 32-bit BitIntType is currently unspecified by the ABI. | |
| // As a conservative approach, pass 32-bit BitIntType as direct without extension. | |
| if (EIT->getNumBits() == 32) | |
| return ABIArgInfo::getDirect(); | |
| if (EIT->getNumBits() < 32) | |
| return extendType(Ty, CGT.ConvertType(Ty)); |
Copilot uses AI. Check for mistakes.
| if (EIT->getNumBits() < XLen) | ||
| return extendType(Ty, CGT.ConvertType(Ty)); | ||
| // FIXME: Maybe we should treat 32 as a special case and wait for | ||
| // the SPEC to decide. |
There was a problem hiding this comment.
| // the SPEC to decide. | |
| // the psABI to decide. |
| // All integral types are promoted to XLen width | ||
| if (Size < XLen && Ty->isIntegralOrEnumerationType()) { | ||
| return extendType(Ty, CGT.ConvertType(Ty)); | ||
| } |
| // FIXME: Maybe we should treat 32 as a special case and wait for | ||
| // the SPEC to decide. | ||
| if (EIT->getNumBits() <= 2 * XLen) | ||
| return ABIArgInfo::getExtend(Ty, CGT.ConvertType(Ty)); | ||
| if (EIT->getNumBits() > 128 || |
There was a problem hiding this comment.
Is the existing code with hasInt128Type, correct for the new ABI?
Implement riscv-non-isa/riscv-elf-psabi-doc#419.
This patch makes the type extension based on the variable type for BIGINT, rather than using sign extension for all cases.