[PATCH] D137108: Implement support for AArch64ISD::MOVI in computeKnownBits

Tue Nov 1 07:29:57 PDT 2022

adriantong1024 added inline comments.

================
Comment at: llvm/test/CodeGen/AArch64/shift-accumulate.ll:124
+
+; Expected to be able to deduce movi is generate a vector of integer
+; and turn USHR+ORR into USRA.
----------------
foad wrote:
> adriantong1024 wrote:
> > foad wrote:
> > > Pre-commit this test and rebase the current patch on top of it?
> > Hi Jay
> > 
> > Thank you for the comment. Do you mean I should change this test and commit it as a separate patch before the actual change (i.e. the implementation of the MOVI) ?
> Yes. Commit the test in a form that will pass on trunk, and then when you rebase this patch, it'll show the difference in the codegen caused by your patch. That makes it much easier for reviewers to see the effect of your patch.
> 
> (I'm not an AArch64 reviewer but this is pretty common practice for the backends I've worked on.)
Makes sense !. I will do so once this patch is accepted. Thanks for teaching me this.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137108