[PATCH] D55444: AMDGPU: Fix DPP combiner

[Valery Pykhtin via Phabricator via llvm-commits]

vpykhtin marked an inline comment as done.
vpykhtin added inline comments.


================
Comment at: lib/Target/AMDGPU/GCNDPPCombine.cpp:476
       BB->insert(OrigMI, NewMI);
       if (TII->commuteInstruction(*NewMI)) {
         LLVM_DEBUG(dbgs() << "  commuted:  " << *NewMI);
----------------
vpykhtin wrote:
> nhaehnle wrote:
> > Can we simplify the code here by just commuting the OrigMI unconditionally?
> > 
> > That is, first check whether `&Use != src0 && &Use == src1`; in that case, try commuting the instruction. If that fails, break (but I don't think there'd be a need to rollback the commutation). If it succeeds, continue down the same path that is used for the `&Use == src0` case. You end up with only a single call to `createDPPInst` and less code duplication in general.
> This is a good idea: I don't like leaving commuted instruction on rollback at some intuitive level but really don't have arguments against it at the moment :-)
Actually a pass returns boolean meaning it changes something in the IR. What should it return when it does rollback? It may create new commuted instructions which are need to be mapped in SlotIndexes for example. I don't think its a good idea to return true only when a rollback took place.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55444