[PATCH] D22247: [DAGCombine] Make sext(setcc) combine respect getBooleanContents()

Chandler Carruth via llvm-commits llvm-commits at lists.llvm.org
Sat Jul 30 15:57:50 PDT 2016


chandlerc added a comment.

In https://reviews.llvm.org/D22247#501500, @mkuper wrote:

> Thanks, Chandler!
>
> Regarding the test case - this is actually somewhat reduced, it's 15 instructions, compared to the 29 in the PR.
>  The reason it's hard to reduce further is that the graph needs to be set up "just so". The combine opportunity needs to be exposed late (post-legalization), because if it happens too soon, you get an i1 SETCC, which doesn't hit the bad codepath. And I don't know of a way to directly generate an i8 (or larger) SETCC from IR.


Ah, sorry if it was already reduced further.

> I'll make a note of this in the test itself, although I'm not sure how helpful it will be. If the combine chain that leads to this being exposed breaks, there's nothing much you can do about that.


Sure, but hopefully with the note some future maintainer will realize that is all that has happened and be able to confidently remove the test because the pattern that it exercised can't be reached any longer, rather than wondering forever if this was a regression or what it means for the test to change. Maybe I'm being optimistic though. ;]


https://reviews.llvm.org/D22247





More information about the llvm-commits mailing list