-
Notifications
You must be signed in to change notification settings - Fork 43
Description
Broadly, the following relaxations have not been implemented correctly, and are invalid:
qc.e.li->c.luiqc.li->c.lui
Both for the same reason:
eld/lib/Target/RISCV/RISCVLDBackend.cpp
Lines 600 to 604 in fde4266
| // For c.lui, we need to be able to relax to C, and we need to have a | |
| // compatible value and destination register. | |
| bool canRelaxCLui = canRelaxXqci && config().options().getRISCVRelaxToC() && | |
| (Value >> 12) != 0 && llvm::isShiftedInt<6, 12>(Value) && | |
| rd != 0 && rd != 2; |
This check does not account for the future value of S+A, only its current value.
Further relaxations may affect the value of S+A, causing the low 12-bits to no longer be all zeros, even though this was the case when the relaxation was applied.
For the same reason, #416 (qc.e.li -> lui) is not correct.
There are two options going forwards:
- Remove both relaxations
- Implement a lot of checks to verify that required alignments mean the low 12-bits will not change after relaxation.
For the 21.x branch (which has this bug), we need to do the first option.
For 22.x, we have more options, but it's not clear that these relaxation would be worth the maintenance burden without these checks.
The workaround before this fix lands is to use --no-relax-c with --relax-xqci.