[PATCH] D30502: [DAGCombiner] fold binops with constant into select-of-constants

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 1 12:47:37 PST 2017


spatel marked an inline comment as done.
spatel added a comment.

In https://reviews.llvm.org/D30502#689854, @efriedma wrote:

> > The debug output for "sel_constants_shl_constant" makes me think that this patch is not at fault for that mess.
>
> I would agree with that assessment; you get the same terrible lowering for "select i1 %cond, i8 -20, i8 115".


Right - I filed a similar example as:
https://bugs.llvm.org/show_bug.cgi?id=32105

> Some targets intentionally fold the other way in certain cases: pulling a math operation out of a select can make the immediates cheaper.  For example, consider "int y; int *a(int x) { return x ? &y : &y+1; }" on ARM.  That isn't directly impacted by this, though, because ARM uses target-specific nodes.

Should I add a target hook or some other means to protect against that case? Or is it ok to assume this is universally good for the currently handled binop types?

> Does it make sense to handle SELECT_CC in addition to SELECT?

I'm only seeing SELECT in the limited scope that I'm looking at so far. I'll add a TODO comment for now if that's ok.


https://reviews.llvm.org/D30502





More information about the llvm-commits mailing list