[PATCH] D81728: [InstCombine] Add target-specific inst combining

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:3781
   auto &TLI = getAnalysis<TargetLibraryInfoWrapperPass>().getTLI(F);
+  auto &TTI = getAnalysis<TargetTransformInfoWrapperPass>().getTTI(F);
   auto &DT = getAnalysis<DominatorTreeWrapperPass>().getDomTree();
----------------
Flakebi wrote:
> lebedev.ri wrote:
> > This opens a dangerous floodgates of instcombine not being target-independent canonicalization pass.
> That is the point of this change, to allow target-dependent combinations in TargetTransformInfo::instCombineIntrinsic.
> Imo, all the target specific intrinsic combinations in InstCombineCalls.cpp (x86, amdgpu, etc.) can be moved to their respective target.
> 
> I don’t have a great overview of LLVM, so I might be wrong on this.
> Imo, all the target specific intrinsic combinations in InstCombineCalls.cpp (x86, amdgpu, etc.) can be moved to their respective target.

I agree with that, yes.

The problem i'm seeing is that even having TTI in the pass
"significantly" lowers the barrier of entry for then using
TTI to guard some generic transforms in the instcombine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81728