[PATCH] Fix a performance problem in gep(gep ...) merging

[Quentin Colombet]

Thanks for the numbers Wei!


REPOSITORY
  rL LLVM

================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:1473
@@ +1472,3 @@
+        // before.
+        if (!isa<Constant>(GO1) || !isa<Constant>(SO1))
+          return nullptr;
----------------
wmi wrote:
> qcolombet wrote:
> > This seems too conservative. In particular, in your comment you had mentioned the number of users.
> > What does a heuristic based on that give?
> At first I planed to add !Src.hasOneUse() in shouldMergeGEPs which disabled all the gep merging if the src has multiple uses. 
> 
>       // Replace: gep (gep %P, long B), long A, ...
>       // With:    T = long A+B; gep %P, T, ...
> 
> Then I think propagation will introduce extra computation only when the add insn (T = long A+B above) generated after the gep merging cannot be deleted. If GO1 and SO1 are both constants, there is no add generated for gep merging, .i.e, in this case, propagation at least introduces no harm.
> 
> So I proposed the patch here. Compared with the first proposal, if there are multiple uses but GO1 and SO1 are constants for some or all the uses, it is still beneficial to do the gep merging fully or partially. However, I saw not much performance difference for both proposals in spec2000 and internal testing.
> 
> I know a more general proposal than the second proposal here is to do gep merging only when the add expr of SO1 and GO1 can be folded into constants. However considering the two proposals have not much perf difference, I didn't try this one. But I can try that if you think it is good to try.
> 
What about something in between:
if ((!isa<Constant>(GO1) || !isa<Constant>(SO1)) && !Src->hasOneUse())

We still keep the benefice when both are constants and we handle single user case.

http://reviews.llvm.org/D8911

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/