This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Insert a sext_inreg when type legalizing i32 shl by constant on RV64.
ClosedPublic

Authored by craig.topper on Aug 24 2021, 2:17 PM.

Details

Summary

Similar to what we do for add/sub/mul.

This can help remove some sext.w. There are some regressions on
some bswap tests, but I have an idea how to fix that for a follow up.

A new PACKW pattern is added to handle the new sext_inreg placement.

Diff Detail

Event Timeline

craig.topper created this revision.Aug 24 2021, 2:17 PM
craig.topper requested review of this revision.Aug 24 2021, 2:17 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 24 2021, 2:17 PM
Herald added a subscriber: MaskRay. · View Herald Transcript
jrtc27 added inline comments.Aug 24 2021, 2:24 PM
llvm/lib/Target/RISCV/RISCVInstrInfoB.td
1140

Does this pattern still get generated? If so it's a shame one isn't canonicalised to the other :(

craig.topper added inline comments.Aug 24 2021, 2:38 PM
llvm/lib/Target/RISCV/RISCVInstrInfoB.td
1140

I think we would still generate it for this

int foo(unsigned long long x, unsigned long long y) {
  return (x << 16) | (y & 0xffff);
}

Type legalization doesn't get involved since x and y are i64. A sign extend will be introduced for the result due to ABI.

In general, we should be pushing sext to the inputs of and/or/xor looking for locations that would make them free. This case could be done with a simple peephole. More complex cases could require looking back through multiple logic ops which could become computationally expensive.

I've posted D108738 to make bitreverse expansion more efficient out of the box. That should prevent the regressions here.

Rebase on top of bitreverse changes from D108738

jrtc27 accepted this revision.Aug 26 2021, 9:57 AM
jrtc27 added inline comments.
llvm/lib/Target/RISCV/RISCVInstrInfoB.td
1140

Yeah, ok

This revision is now accepted and ready to land.Aug 26 2021, 9:57 AM
This revision was landed with ongoing or failed builds.Aug 26 2021, 10:20 AM
This revision was automatically updated to reflect the committed changes.