[PATCH] D132325: [AArch64][CodeGen] Fold the mov and lsl into ubfiz

Tue Aug 23 12:23:21 PDT 2022

efriedma added inline comments.

================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:7348
 def : Pat<(shl (i64 (sext GPR32:$Rn)), (i64 imm0_63:$imm)),
-          (SBFMXri (INSERT_SUBREG (i64 (IMPLICIT_DEF)), GPR32:$Rn, sub_32),
+          (SBFMXri (SUBREG_TO_REG (i64 0), GPR32:$Rn, sub_32),
+                   (i64 (i64shift_a        imm0_63:$imm)),
----------------
paulwalker-arm wrote:
> @efriedma / @Allen : Sorry for the naive question but I'm not hugely familiar with this node and the documentation says:
> ```
> /// All other bits are
> /// assumed to be equal to the bits in the immediate integer constant in the
> /// first operand. This instruction just communicates information; No code
> /// should be generated.
> ```
> So is this safe? I mean if no code is generated then in this instance how can we be sure `$Rn` has it's top 32bits zeroed? I'm kind of assuming this is why the original code is using `INSERT_SUBREG`?
> 
> The emitted code is valid, but could something query the `SUBREG_TO_REG` post isel and reuse/transform it in an invalid way?
Oh, I didn't notice it was changed from using INSERT_SUBREG to use SUBREG_TO_REG.  I just glanced over the patterns and assumed it was just using the same pattern we were already using.  Not sure why it was changed.  We normally only use SUBREG_TO_REG if we know the operand actually clears the high bits.

Not sure there's really any practical consequence here; there isn't very much code that's actually aware of the semantics of SUBREG_TO_REG.

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

https://reviews.llvm.org/D132325