[PATCH] D61289: [globalisel] Add G_SEXT_INREG

[Daniel Sanders via Phabricator via llvm-commits]

dsanders marked 2 inline comments as done.
dsanders added inline comments.


================
Comment at: llvm/test/CodeGen/AArch64/GlobalISel/legalize-sext.mir:11-12
+    ; CHECK-DAG: [[SEXT1:%[0-9]+]]:_(s32) = G_SHL [[COPY]], [[I25]]
+    ; This constant is coming from a custom legalization for G_ASHR rather than G_SEXT_INREG lowering
+    ; CHECK-DAG: [[I25_64:%[0-9]+]]:_(s64) = G_CONSTANT i64 25
+    ; CHECK-DAG: [[SEXT2:%[0-9]+]]:_(s32) = G_ASHR [[SEXT1]], [[I25_64]]
----------------
aemerson wrote:
> dsanders wrote:
> > @paquette @aemerson: I ought to draw attention to this. My code appears to be doing the right thing for lower() of G_SEXT_INREG but you appear to have a custom legalization that promotes one of the two constants to s64. Is that intentional? If so, is it also intentional that it only does it for G_ASHR and not G_SHL too?
> Yes this is intentional. In order to re-use the existing imported patterns for ashr & lshr we promote the shift amount to i64. For G_SHL we have some custom selection code to deal with non-64b immediates so it's not necessary.
That's ok then. Thanks


================
Comment at: llvm/test/CodeGen/AArch64/GlobalISel/legalize-undef.mir:29
+    ; CHECK-DAG: [[DEF:%[0-9]+]]:_(s64) = G_IMPLICIT_DEF
+    ; CHECK-DAG: [[SHL:%[0-9]+]]:_(s64) = G_SHL [[DEF]], [[C]](s64)
+    ; CHECK-DAG: [[ASHR:%[0-9]+]]:_(s64) = G_ASHR [[SHL]], [[C]](s64)
----------------
aemerson wrote:
> Did constant and impdef order change? If so we can just re-run the test update script.
> Did constant and impdef order change?
Yes.

Should instruction order matter for these tests? Legalization doesn't give any guarantees on the output order and using CHECK-DAG makes the test robust against that


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61289