[AIEX] Refactor Set/GetControlRegister and UPS instruction selection to share common code #712
Conversation
abhinay-anubola
requested review from
F-Stuckmann,
SagarMaheshwari99,
abnikant,
andcarminati,
katerynamuts,
khallouh,
konstantinschwarz,
martien-de-jong,
mludevid,
niwinanto and
stephenneuendorffer
as code owners
| case AIE2::crF2FMask: | ||
| case AIE2::crF2IMask: | ||
| case AIE2::crFPMask: | ||
| return SrcConstVal % (1 << 5); |
There was a problem hiding this comment.
Perhaps we can generalize this using getRegSizeInBits(Reg, MRI)
There was a problem hiding this comment.
Hmm. It looks as if their size is 32 bit...
There was a problem hiding this comment.
Yes, you're right getRegSizeInBits would return 32. However, this function needs the semantic bit width.
| // the register according to register width. | ||
| switch (CtrlReg) { | ||
| case AIE2::crSat: | ||
| return SrcConstVal % (1 << 2); |
There was a problem hiding this comment.
CHECK: this is mimicking the hardware when it would move it from a register source. To be frank, I would prefer a frontend error on writing an out of bound constant value.
There was a problem hiding this comment.
Where should the valid range constraints for control registers be enforced, at the intrinsic definition in headers?
| // Check if the argument is constant for register map index. | ||
| const auto Idx = getIConstantVRegValWithLookThrough(IdxReg, MRI); | ||
| if (!Idx) | ||
| llvm_unreachable("Expected const value for control register map index."); |
There was a problem hiding this comment.
getIConstantVRegValWithLookThroughOrFail() could encapsulate the llvm_unreachable.
Also support get/set_status_reg in instruction selection for AIE2P