[llvm-commits] [llvm] r60857 - in /llvm/trunk: lib/CodeGen/SelectionDAG/DAGCombiner.cpp test/CodeGen/X86/add-with-overflow.ll

[Eli Friedman]

On Thu, Dec 11, 2008 at 12:23 AM, Bill Wendling <isanbard at gmail.com> wrote:
> On Dec 10, 2008, at 3:44 PM, Eli Friedman wrote:
>
>> On Wed, Dec 10, 2008 at 2:36 PM, Bill Wendling <isanbard at gmail.com>
>> wrote:
>>> Author: void
>>> Date: Wed Dec 10 16:36:00 2008
>>> New Revision: 60857
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=60857&view=rev
>>> Log:
>>> If ADD, SUB, or MUL have an overflow bit that's used, don't do
>>> transformation on
>>> them. The DAG combiner expects that nodes that are transformed have
>>> one value
>>> result.
>>
>> This approach somehow seems a little scary; I'm not completely sure,
>> but do we really want target-independent nodes producing
>> platform-specific results?  I don't think any other code does this,
>> and it seems like it's overloading the semantics in a dangerous way.
>> Is there some particular reason why you preferred this over adding
>> instructions like X86ISD::ADD_FLAGS for an add that sets EFLAGS?
>>
> The idea is that there be platform independent ways of calculating
> when an overflow has occurred, and rely upon target-specific lowering
> when the target can handle the flags. This is what ADDO and SUBO do.
> MULO is trickier, so it doesn't do that . . . yet.

I'm aware of all of that, and it's not really what I was asking about.
My question is, why are you using the second output value of ISD::ADD?
 Per the platform-independent definition, the output you're using
doesn't exist; the fact that it gets lowered to the expected result
is, as far as I can tell, essentially a quirk in the way instruction
selection works.  Also, no other existing code uses a pattern like
this.

-Eli