[PATCH] D50633: [AMDGPU] Add new Mode Register pass

[Nicolai Hähnle via Phabricator via llvm-commits]

nhaehnle added a comment.

One last thing. If I'm right about this, it'd be good to reduce the check complexity and indentation levels. Then I'm happy :)



================
Comment at: lib/Target/AMDGPU/SIModeRegister.cpp:243-246
+      if (NewInfo->Change.isCompatible(InstrMode)) {
+        // This instruction is compatible, so just merge the mode
+        NewInfo->Change = NewInfo->Change.merge(InstrMode);
+      } else {
----------------
timcorringham wrote:
> nhaehnle wrote:
> > nhaehnle wrote:
> > > Hasn't this become redundant now?
> > Okay, no, I see how it isn't redundant if there's a pre-existing s_setreg.
> > 
> > But why the call to merge? isCompatible only returns true if InstrMode is a subset of the known bits of NewInfo->Change.
> Good spot- this is now redundant. It is benign but unnecessary - I'll remove it.
Nice. Isn't the `InstrMode != Status()` check now also redundant, actually?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D50633