[PATCH] D76354: [RISCV][GlobalISel] Legalize types for ALU operations

Mon Mar 14 09:03:43 PDT 2022

lewis-revill added inline comments.
Herald added a project: All.

================
Comment at: llvm/lib/Target/RISCV/RISCVLegalizerInfo.cpp:101-104
+bool RISCVLegalizerInfo::legalizeWOpWithSExt(
+    MachineInstr &MI, MachineRegisterInfo &MRI,
+    MachineIRBuilder &MIRBuilder) const {
+  const LLT s64 = LLT::scalar(64);
----------------
arsenm wrote:
> lewis-revill wrote:
> > arsenm wrote:
> > > You shouldn't have this function, you're just repeating totally ordinary promotion the legalizer will handle by default
> > It doesn't do this by default though. By default the destination gets widened without a `G_SEXT` or a `G_ZEXT`, so any pattern we could look to select later on ends up disappearing. I can use the `widenScalarSrc` as a helper but the `widenScalarDst` employed by the default lowering will give the wrong behaviour.
> It does if you set your legalization rules to promote these operations. The new result type is the requested promoted type, with a truncate to the original register. The SextInReg here is unnecessary, and also doesn't really do anything since you're truncating right back to the original result type, discarding the extended bits. 
Sorry I'm still not getting it. How do I set legalization rules to promote these operations? The only similar options I see are `minScalar`/`clampScalar` which will destroy any pattern we could hope to select? Might you be able to explain more?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76354/new/

https://reviews.llvm.org/D76354