On Fri, Jan 27, 2017 at 10:30 AM, Jeff Law <l...@redhat.com> wrote:
> On 01/27/2017 05:08 AM, Richard Biener wrote:
>>
>> On Fri, Jan 27, 2017 at 10:02 AM, Marc Glisse <marc.gli...@inria.fr>
>> wrote:
>>>
>>> On Thu, 26 Jan 2017, Jeff Law wrote:
>>>
>>>>> I assume this causes a regression for code like
>>>>>
>>>>> unsigned f(unsigned a){
>>>>> unsigned b=a+1;
>>>>> if(b<a)return 42;
>>>>> return b;
>>>>> }
>>>>
>>>>
>>>> Yes. The transformation ruins the conversion into ADD_OVERFLOW for the
>>>> +-
>>>> 1 case. However, ISTM that we could potentially recover the
>>>> ADD_OVERFLOW in
>>>> phi-opt. It's a very simple pattern that would be presented to phi-opt,
>>>> so
>>>> it might not be terrible to recover -- which has the advantage that if a
>>>> user wrote an optimized overflow test we'd be able to recover
>>>> ADD_OVERFLOW
>>>> for it.
>>>
>>>
>>>
>>> phi-opt is a bit surprising at first glance because there can be overflow
>>> checking without condition/PHI, but if it is convenient to catch many
>>> cases...
>>
>>
>> Yeah, and it's still on my TODO to add some helpers exercising
>> match.pd COND_EXPR
>> patterns from PHI nodes and their controlling condition.
>
> It turns out to be better to fix the existing machinery to detect
> ADD_OVERFLOW in the transformed case than to add new detection to phi-opt.
I had a start on improving PHI-OPT to use match.pd directly. I
started a prototype patch to
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25290 . It needs many
cleanups which I hope to do early March. As I said in the bug report,
the patch started out on as doing conditional move; I disabled that
part but did not remove it. This is part of the cleanup which I hope
to do. There is some interesting interactions with doing the match.pd
part on generic which I did not have time to debug yet either. Though
it was able to do simple simplifications using match.pd now. I hope
to clean up the patch and finish it up even removing the more complex
parts of phi-opt too.
Thanks,
Andrew
>
> The problem with improving the detection of ADD_OVERFLOW is that the
> transformed test may allow the ADD/SUB to be sunk. So by the time we run
> the pass to detect ADD_OVERFLOW, the test and arithmetic may be in different
> blocks -- ugh.
>
> The more I keep thinking about this the more I wonder if transforming the
> conditional is just more of a headache than its worth -- the main need here
> is to drive propagation of known constants into the THEN/ELSE clauses.
> Transforming the conditional makes that easy for VRP & DOM to discover those
> constant and the transform is easy to write in match.pd. But we could just
> go back to discovering the case in VRP or DOM via open-coding detection,
> then propagating the known constants without transforming the conditional.
>
> Jeff
