[PATCH] critical-anti-dependency breaker: don't use reg def info from kill insts (PR20308)

Sanjay Patel spatel at rotateright.com
Wed Aug 20 10:34:57 PDT 2014


>>! In D4977#16, @hfinkel wrote:
> I think that we might still want to process Kill instructions that have a subregister/subregister relationship. These are the kinds of 
> kills that the code is expecting, and there is logic to group these and rename them together. We probably don't want to 
> completely disable that functionality. Does that makes sense?

Yes, that makes sense for the Aggressive antidep breaker. I hadn't looked at the Aggressive implementation until now.

I don't see any handling for kills in the Critical antidep breaker though. These 2 implementations seem quite different at first glance. I can add a note in the code comments for this patch to also consider the behavior in the Aggressive class.


> When we commit a fix to this, please also revert r214429 (which was my related temporary fix to a subset of this problem).

Sure - I posted the testcase in this patch into bug 18663 with my basic analysis of what's happening in MCP. 

Just to be clear, do you approve this patch (with some changes to the comments) as a temporary fix to avoid the miscompile?

 
> P.S. In the future, please make sure to upload full-context diffs (see the instructions here: http://llvm.org/docs/Phabricator.html).

Sorry - will do.

http://reviews.llvm.org/D4977






More information about the llvm-commits mailing list