[PATCH] Fix PR16938 - rewrite CSE heuristic for same block uses

Andrew Trick atrick at apple.com
Tue Apr 29 17:32:15 PDT 2014


On Apr 29, 2014, at 12:38 PM, Eric Christopher <echristo at gmail.com> wrote:

> Ping :)
> 
> -eric
> 
> On Tue, Apr 22, 2014 at 4:21 PM, Eric Christopher <echristo at gmail.com> wrote:
>> Hi Evan,
>> 
>> In taking a look at 16938 it seems that what you want is to check the
>> uses of the original register in all of the basic blocks and then make
>> sure that the new CSE'd expression is in one of them rather than the
>> inverse?
>> 
>> This will fix the PR mentioned above and no regressions anywhere. I
>> ran the performance tests and didn't see anything outside of the noise
>> as well.
>> 
>> Thoughts?

I can guess what the old heuristic was doing, but we don’t have a test case for it so I’m only speculating.

Your new heuristic is totally different. For your tests it would be just as good to simply delete the original heuristic. So we’re left with the question, why does your new heuristic do anything good (it doesn’t match the comments and it’s nonobvious)? Do we have any test case or performance results that show goodness?

In this situation, I would delete the old heuristic, see what regresses, then try to fix it with a new one.

-Andy



More information about the llvm-commits mailing list