[PATCH] D89823: [AArch64][GlobalISel] Move imm adjustment for G_ICMP to post-legalizer lowering

[Amara Emerson via Phabricator via llvm-commits]

aemerson added inline comments.


================
Comment at: llvm/test/CodeGen/AArch64/GlobalISel/select-arith-immed-compare.mir:31
-    %1:gpr(s32) = G_CONSTANT i32 4097
-    %4:gpr(s32) = G_ICMP intpred(slt), %0(s32), %1
-    %5:gpr(s32) = G_CONSTANT i32 1
----------------
paquette wrote:
> aemerson wrote:
> > Can we have some end-to-end tests that show that the MIR that we previously had being selected to CSINCWr still selects to the same output if we now go through lowering->select? Don't have to cover all the cases but to show the mechanism still works.
> > 
> > Maybe we'll need to use .ll tests for this, not sure.
> To avoid a .ll test we could do something like this:
> 
> ```
>  llc -mtriple=aarch64 -start-before=aarch64-postlegalizer-lowering -stop-after=instruction-select -verify-machineinstrs
> ```
> 
> A slightly hackier, but more pointed version would be:
> 
> ```
>  llc -mtriple=aarch64 -run-pass=aarch64-postlegalizer-lowering -run-pass=instruction-select -verify-machineinstrs
> ```
> 
> (IIRC this works)
> 
> From the perspective that we'd only run aarch64-postlegalizer-lowering and instruction-select, I guess this is better. We'd have to use regbankselected MIR though, which is a little wonky. Not sure if that's something that we'd want to support in the future.
> 
> Preferences? (I could also just take the .ll test route, or add a GISel checkline to an existing .ll test somewhere probably)
I didn't realize that -start-before and -stop-after actually worked without crashing. That's the ideal route, otherwise adding check lines to an existing .ll is fine too.


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

https://reviews.llvm.org/D89823