[llvm-dev] Problem ScheduleDAG on PowerPC, X86 works fine.

Thu Feb 9 02:21:27 PST 2017

So the plan would be:

1/ Use UADDO instead of ADDC.
2/ Deprecate and remove ADDC.
3/ Have ADDE/SUBE accept a boolean as carry input instead of a glue.

There are still one thing I don't really understand: how come the X86
backend end up avoiding turnning into the problem altogether to begin with.
I think that's an important piece of the puzzle.

2017-02-09 5:48 GMT+01:00 James Y Knight <jyknight at google.com>:

> I'd think i1 would be the proper and correct choice for a carry flag for
> the generic instruction. I expect that would also make UADDO/USUBO
> redundant with ADDC/SUBC (which would seem a good outcome).
>
> You'd need to make sure the right thing happened when converting from
> ADDC's 1-bit carry in/out to X86ISD::AD[DC]'s EFLAGS i/o. Right now the
> conversion can get away with assuming that the only thing which can be used
> as input to ADDC is the glue-output of ADD/ADDC. That would no longer be
> the case once they're using a normal value.
>
> (Actually, I wonder if it would be possible to represent x86's
> EFLAGS.CARRY as a 1-bit subreg of EFLAGS, and use that on input to
> X86ISD::ADC; that may allow cheaper spills/reloads if it only has to save
> the one bit?).
>
> On Wed, Feb 8, 2017 at 7:28 PM, Amaury SECHET <deadalnix at gmail.com> wrote:
>
>> Well I don't think this would break most backend. The opcode is generate
>> only if the backend allows so to boot, so many backend actually do not use
>> it at all.
>>
>> Anyway, changing these opcode is kind of much broader scope than what I
>> anticipated, but I maybe can make it work. What do you think would be an
>> appropriate type for the carry instead of glue ?
>>
>> 2017-02-08 21:19 GMT+01:00 James Y Knight <jyknight at google.com>:
>>
>>> I don't think that'd work, because it leaves all other backends broken.
>>> AFAICT, your transform is simply not a legal transform, with the way the
>>> ADDC/ADDE opcodes are currently defined, and to do it you really need to
>>> fix the opcode definitions to not involve glue, first.
>>>
>>> I also note that your transform doesn't actually trigger at all on this
>>> particular test case on x86, because the dag ends up looking like (uaddo X,
>>> (adde Y, 0, Carry)), which the transform doesn't match. That is not really
>>> relevant to the issue here, because the x86 backend does work even if the
>>> transform gets triggered (replacing the final store with "store i64 %add37,
>>> i64* %r, align 8" will make this happen)
>>>
>>>
>>> On Tue, Feb 7, 2017 at 5:44 PM, Amaury SECHET <deadalnix at gmail.com>
>>> wrote:
>>>
>>>> Making this a value instead of a glue looks like a good longer term
>>>> solution, but it doesn't quite cut it as a short term one. It would require
>>>> to change a fair amount of code in the DAG stack, plus in each backend.
>>>>
>>>> @James,
>>>> do you think doing something similar to what the X86 backend does in
>>>> the PowerPC one would cut it ?
>>>>
>>>>
>>>> 2017-02-07 22:51 GMT+01:00 Nemanja Ivanovic <nemanja.i.ibm at gmail.com>:
>>>>
>>>>> Would it not make sense to refactor the code so those don't use glue
>>>>> rather than emitting them with glue and then getting rid of it. There are
>>>>> times when we would like to emit these in separate blocks but can't
>>>>> (presumably because of the glue).
>>>>>
>>>>> On Tue, Feb 7, 2017 at 9:15 PM, James Y Knight via llvm-dev <
>>>>> llvm-dev at lists.llvm.org> wrote:
>>>>>
>>>>>> That's seems really odd that ADDC/ADDE uses glue there, instead of a
>>>>>> plain value.
>>>>>>
>>>>>> The x86 backend has code that converts the glue into a value, which
>>>>>> is why it wasn't affected.... (LowerADDC_ADDE_SUBC_SUBE).
>>>>>>
>>>>>> On Tue, Feb 7, 2017 at 2:48 PM, Amaury SECHET via llvm-dev <
>>>>>> llvm-dev at lists.llvm.org> wrote:
>>>>>>
>>>>>>> Long story short: https://llvm.org/bugs/show_bug.cgi?id=31890
>>>>>>>
>>>>>>> The backend fails to schedule a given DAG, the reason being that
>>>>>>> there is an instruction and it glue that needs to be broken apart as they
>>>>>>> can't be scheduled consecutively. See attached file for a picture of the
>>>>>>> DAG.
>>>>>>>
>>>>>>> Not sure what's the best course of action is, and not sure why this
>>>>>>> isn't a problem for the X86 backend either. I'm looking for advice on the
>>>>>>> best course of actions. As I see it, the option are:
>>>>>>>
>>>>>>> 1/ add extra logic in the DAGCombiner to make sure this doesn't
>>>>>>> happen. I don't see a way this could be done cheaply and overall I don't
>>>>>>> think this is the best option/
>>>>>>> 2/ Have the ScheduleDAG machinery detect this case and break up the
>>>>>>> glue, for instance via breaking up (adde X, Y, Carry) into (add (add X, Y)n
>>>>>>> (adde 0, 0, Carry)) or something alike when the situation present itself.
>>>>>>> 3/ Do whatever the X86 backend does, which I'm not sure what it is.
>>>>>>>
>>>>>>> Advice ?
>>>>>>>
>>>>>>> Thanks in advance,
>>>>>>>
>>>>>>> Amaury SECHET
>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> LLVM Developers mailing list
>>>>>>> llvm-dev at lists.llvm.org
>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> LLVM Developers mailing list
>>>>>> llvm-dev at lists.llvm.org
>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170209/3134b20b/attachment-0001.html>