[PATCH] D149577: [InstCombine] Improve bswap optimization

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added inline comments.


================
Comment at: llvm/test/Transforms/InstCombine/bswap-fold.ll:542
 
+; Issue#62236
+; Fold: BSWAP( OP( BSWAP(x), y ) ) -> OP( x, BSWAP(y) )
----------------
goldstein.w.n wrote:
> austin880625 wrote:
> > goldstein.w.n wrote:
> > > Can you split the new tests to a seperate patch so we can see the diff generated?
> > > 
> > Just to confirm(newbie here), are the following separated patches you want:
> > 
> > 1. first one in old implementation and new tests with FileCheck lines generated with old `opt`
> > 2. second one with my actual code change, and the new tests with updated FileCheck lines generated with the changed opt. And the patch is based diff-ed with the first one?
> > Just to confirm(newbie here), are the following separated patches you want:
> > 
> > 1. first one in old implementation and new tests with FileCheck lines generated with old `opt`
> > 2. second one with my actual code change, and the new tests with updated FileCheck lines generated with the changed opt. And the patch is based diff-ed with the first one?
> 
> Yeah. You can create series using the 'edit related revisions' then make the test-patch the "parent".
> For example:
> Tests: D149520
> Impl: D149521
> 
> and if click 'stack' you see the relationship between the two.
> > Just to confirm(newbie here), are the following separated patches you want:
> > 
> > 1. first one in old implementation and new tests with FileCheck lines generated with old `opt`
> > 2. second one with my actual code change, and the new tests with updated FileCheck lines generated with the changed opt. And the patch is based diff-ed with the first one?
> 
> Yeah. You can create series using the 'edit related revisions' then make the test-patch the "parent".
> For example:
> Tests: D149520
> Impl: D149521
> 
> and if click 'stack' you see the relationship between the two.




================
Comment at: llvm/test/Transforms/InstCombine/bswap-fold.ll:542
 
+; Issue#62236
+; Fold: BSWAP( OP( BSWAP(x), y ) ) -> OP( x, BSWAP(y) )
----------------
goldstein.w.n wrote:
> goldstein.w.n wrote:
> > austin880625 wrote:
> > > goldstein.w.n wrote:
> > > > Can you split the new tests to a seperate patch so we can see the diff generated?
> > > > 
> > > Just to confirm(newbie here), are the following separated patches you want:
> > > 
> > > 1. first one in old implementation and new tests with FileCheck lines generated with old `opt`
> > > 2. second one with my actual code change, and the new tests with updated FileCheck lines generated with the changed opt. And the patch is based diff-ed with the first one?
> > > Just to confirm(newbie here), are the following separated patches you want:
> > > 
> > > 1. first one in old implementation and new tests with FileCheck lines generated with old `opt`
> > > 2. second one with my actual code change, and the new tests with updated FileCheck lines generated with the changed opt. And the patch is based diff-ed with the first one?
> > 
> > Yeah. You can create series using the 'edit related revisions' then make the test-patch the "parent".
> > For example:
> > Tests: D149520
> > Impl: D149521
> > 
> > and if click 'stack' you see the relationship between the two.
> > > Just to confirm(newbie here), are the following separated patches you want:
> > > 
> > > 1. first one in old implementation and new tests with FileCheck lines generated with old `opt`
> > > 2. second one with my actual code change, and the new tests with updated FileCheck lines generated with the changed opt. And the patch is based diff-ed with the first one?
> > 
> > Yeah. You can create series using the 'edit related revisions' then make the test-patch the "parent".
> > For example:
> > Tests: D149520
> > Impl: D149521
> > 
> > and if click 'stack' you see the relationship between the two.
> 
> 
And welcome! Thank you for the patch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149577