[PATCH] D32241: Don't process debug intrinsics in InstCombine

[Dmitry Mikulin via Phabricator via llvm-commits]

dmikulin added inline comments.


================
Comment at: include/llvm/Transforms/InstCombine/InstCombineWorklist.h:64
     for (Instruction *I : reverse(List)) {
-      WorklistMap.insert(std::make_pair(I, Idx++));
-      Worklist.push_back(I);
+      if (!isa<DbgInfoIntrinsic>(I)) {
+        WorklistMap.insert(std::make_pair(I, Idx++));
----------------
davide wrote:
> craig.topper wrote:
> > Should you skip it when the List being passed in here is being populated?
> I'd hoist the insertion in the working list in a separate helper so that we don't have to copy the check in multiple places.
Passing in the initial list from AddReachableCodeToWorklist() without @llvm.dbg works too. I'll make the change.
Do we even need the check in Add() if the initial work list has no @llvm.dbg?


================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:3074-3075
 
-      InstrsForInstCombineWorklist.push_back(Inst);
+      // Skip processing intrinsics in InstCombine
+      if (!isa<DbgInfoIntrinsic>(Inst))
+        InstrsForInstCombineWorklist.push_back(Inst);
----------------
davide wrote:
> Please expand this comment to explain why we want to skip them.
      // Skip processing debug intrinsics in InstCombine. Processing these call instructions
      // consumes non-trivial amount of time and provides no value for the optimization.

Is this OK?


================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:3074-3075
 
-      InstrsForInstCombineWorklist.push_back(Inst);
+      // Skip processing intrinsics in InstCombine
+      if (!isa<DbgInfoIntrinsic>(Inst))
+        InstrsForInstCombineWorklist.push_back(Inst);
----------------
craig.topper wrote:
> dmikulin wrote:
> > davide wrote:
> > > Please expand this comment to explain why we want to skip them.
> >       // Skip processing debug intrinsics in InstCombine. Processing these call instructions
> >       // consumes non-trivial amount of time and provides no value for the optimization.
> > 
> > Is this OK?
> I'm really sorry to keep asking this to be moved, but is there any reason to run any of the code above here for debug intrinsics?
I'm not sure how far up I can move the check. Top of the for loop? But won't we potentially miss DCE and constant folds above?


https://reviews.llvm.org/D32241