Hi Marcus: Thank you for digging this issue out, I would suggest you sent v4 patch which only v3 + riscv_subword fix, and then merge into master first, and then sent separate patch for that issue, not sure what your fix, but I guess it might fix some code for IRA/LRA, so I think has a separate patch would be easy to discussion with other (non-RISC-V) maintainers.
On Mon, Mar 15, 2021 at 5:42 AM Marcus Comstedt <mar...@mc.pp.se> wrote: > > > Hello again Kito. > > I've now delved a bit deeper into the failure of the testcase > gcc.c-torture/compile/pr35318.c on big endian RV32. > > The point at which big endian diverges from little endian is where > process_alt_operands() is processing the "%0" constraint. It calls > operands_match_p(), which succeeds on little endian but fails on > big endian. > > On little endian, the two rtx:es passed to operands_match_p are > "r79:DF#0" and "r79:DF", while on big endian they are "r79:DF#4" and > "r79:DF". While the first operand is different, it's actually saying > the same thing: The subreg with the least significant bits (meaning > the second register in the pair on big endian, and the first register > in the pair on little endian, what with two 32-bit integer registers > being allocated to hold a single 64-bit float). > > The helper function lra_constraint_offset(), which is used by > operands_match_p, seems to be intended to handle this discrepancy. It > contains the code > > if (WORDS_BIG_ENDIAN > && is_a <scalar_int_mode> (mode, &int_mode) > && GET_MODE_SIZE (int_mode) > UNITS_PER_WORD) > return hard_regno_nregs (regno, mode) - 1; > > However, in this case the rule does not trigger because the mode of > the second operand (which is the one where an adjustment would be > needed) does not have a scalar_int_mode, it has DFmode. If I relax > this code to also allow scalar_float_mode, then the operands_match_p > call succeeds also on big endian. There is still an ICE triggered > further down the line though. > > I seem to be finding more questions than answers here. Questions such > as "is it really correct that the first operand to operands_match_p() > has modeSI but the second one has modeDF?", "_should_ the operands > match?", and "why is the least significant half singled out when there > is no computation being perfomed". > > Given that the code generated for LE seems incorrect, I still suspect > that there is some deeper issue here not related to endianness (but > possibly related to using integer registers for passing floating point > values to/from asm statements) and that it just happens to not cause > an internal error (only bad code) on LE. > > How would you like to proceed? I don't feel confident that I will > find a definitive solution to this issue anytime soon, but it feels > like such a weird special case (who passes 64-bit floats in 32-bit > integer registers to their asm?) that it might be ok to just ignore > it. If you agree I'll just repost the patchset with the final fix > added (solves all remaining 32-bit testcases save for this one)... > > > // Marcus > >
