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

[Evan Cheng]

On Dec 11, 2008, at 1:13 AM, Eli Friedman wrote:

> 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.

We're going to fix this soon.

Evan

>
>
> -Eli
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits