Target dependence of conditional expression gimplification

Target dependence of conditional expression gimplification

[Florian Weimer]

I have code like this:

      if (auth_info->attrs.pin.flags & SC_PKCS15_PIN_FLAG_SO_PIN) {
         preferred = 1;
     } else {
         preferred = current;
         /* PINs are even numbered, PUKs are odd */
         if (!(preferred & 1))
             preferred++;
         if (preferred >= 126)
             return SC_ERROR_TOO_MANY_OBJECTS;
     }

     if (current > preferred || preferred > CARDOS_PIN_ID_MAX)
         return SC_ERROR_TOO_MANY_OBJECTS;

On x86_64, the condition in the final if statement is gimplified as:

-  D.8003 = current > preferred;
-  D.8004 = preferred > 15;
-  D.8005 = D.8003 | D.8004;
-  if (D.8005 != 0) goto <D.8006>; else goto <D.8007>;

On rs6000 (-mtune=power7 -mcpu=power7), I get this instead:

+  if (current > preferred) goto <D.8376>; else goto <D.8378>;
+  <D.8378>:
+  if (preferred > 15) goto <D.8376>; else goto <D.8377>;

Obviously, this affects optimization-dependent warnings later in the 
compilation.

I wonder why this happens.  Shouldn't gimplification be roughly 
target-independent?

-- 
Florian Weimer / Red Hat Product Security Team

Re: Target dependence of conditional expression gimplification

[Florian Weimer]

On 03/03/2014 01:03 PM, Florian Weimer wrote:
> I have code like this:
>
>       if (auth_info->attrs.pin.flags & SC_PKCS15_PIN_FLAG_SO_PIN) {
>          preferred = 1;
>      } else {
>          preferred = current;
>          /* PINs are even numbered, PUKs are odd */
>          if (!(preferred & 1))
>              preferred++;
>          if (preferred >= 126)
>              return SC_ERROR_TOO_MANY_OBJECTS;
>      }
>
>      if (current > preferred || preferred > CARDOS_PIN_ID_MAX)
>          return SC_ERROR_TOO_MANY_OBJECTS;
>
> On x86_64, the condition in the final if statement is gimplified as:
>
> -  D.8003 = current > preferred;
> -  D.8004 = preferred > 15;
> -  D.8005 = D.8003 | D.8004;
> -  if (D.8005 != 0) goto <D.8006>; else goto <D.8007>;
>
> On rs6000 (-mtune=power7 -mcpu=power7), I get this instead:
>
> +  if (current > preferred) goto <D.8376>; else goto <D.8378>;
> +  <D.8378>:
> +  if (preferred > 15) goto <D.8376>; else goto <D.8377>;

Here's a self-contained reproducer:

int g(void);

int
f(int flags, int current)
{
   int preferred;
   if (flags & 0x80) {
     preferred = 1;
   } else {
     preferred = current;
     if (!(preferred & 1))
       preferred++;
     if (preferred >= 126)
       return 55;
   }

   if (current > preferred || preferred > 17)
     return 55;
   return g();
}

> Obviously, this affects optimization-dependent warnings later in the
> compilation.
>
> I wonder why this happens.  Shouldn't gimplification be roughly
> target-independent?

It turns out that it's not gimplification, but fold which is the 
culprit.  In fold-const.c:fold_truth_andor, we have this code:

   if (LOGICAL_OP_NON_SHORT_CIRCUIT
       && (code == TRUTH_AND_EXPR
           || code == TRUTH_ANDIF_EXPR
           || code == TRUTH_OR_EXPR
           || code == TRUTH_ORIF_EXPR))
     {
Â…
       /* Transform (A AND-IF B) into (A AND B), or (A OR-IF B)
	 into (A OR B).
	 For sequence point consistancy, we need to check for trapping,
	 and side-effects.  */
       else if (code == icode && simple_operand_p_2 (arg0)
                && simple_operand_p_2 (arg1))
	return fold_build2_loc (loc, ncode, type, arg0, arg1);

rs6000 defines LOGICAL_OP_NON_SHORT_CIRCUIT as 0, so this code never 
runs, and the gimplifier produces two separate if statements.

-- 
Florian Weimer / Red Hat Product Security Team

Re: Target dependence of conditional expression gimplification

[Manuel López-Ibáñez]

On 5 May 2014 17:05, Florian Weimer <fweimer@redhat.com> wrote:

>> Obviously, this affects optimization-dependent warnings later in the
>> compilation.
>>
>> I wonder why this happens.  Shouldn't gimplification be roughly
>> target-independent?
>
>
> It turns out that it's not gimplification, but fold which is the culprit.
> In fold-const.c:fold_truth_andor, we have this code:

This is a known issue. See PR32643, PR60090 and specially
http://gcc.gnu.org/ml/gcc/2013-11/msg00253.html

And this quote from http://gcc.gnu.org/ml/gcc-patches/2008-10/msg01061.html

>>>
At present, there are still various "optimizations" done in
the C front end while the trees for expressions are built up, and some
cases where some folding still happens; such optimizations can be
incrementally moved out of the front end into fold (or other
optimizers) where they belong, and such folding during parsing
incrementally disabled, moving towards trees that more closely
correspond to the original source.

In addition, the front end does not logically need all the
transformations done by fold.  In principle, fold should act on GIMPLE
(so avoiding any present dependence on which subexpressions were
combined into larger expressions in the source program) with the only
folding done before gimplification being more limited folding required
for initializers.  With such a change, c_fully_fold would only need to
do the more limited folding.  If the C gimplification handled
C_MAYBE_CONST_EXPR, some calls to c_fully_fold could be eliminated
altogether; only those where the information about constancy is
required, in particular for static initializers, would need to remain.

However, c_fully_fold could also be thought of as a logical lowering
step, converting front-end-specific structures (which presently are
GENERIC plus the odd extra tree code) to GENERIC, with potentially
further transformations needed in future, and increases in the amount
of lowering involved (making datastructures correspond more closely to
the source for longer in the front end) might require this lowering
step everywhere even when folding isn't needed.
<<<

I'm not sure if Marek Polacek has started working on it but he
probably will appreciate some help.

I don't think there is even a plan or TODO list written up of what
needs to be done. It would be nice to create that in the wiki and
check with Joseph, Jason and the middle-end maintainers that this plan
matches their expectations. My understanding at the moment is that:

* Optimizations should be moved from the FEs to middle-end (or to the
gimplification phase). There is the suspicion that many are already
redundant or actually undone by the middle-end, but someone needs to
go over each of them and verify this.

* There should be a FE-specific fold limited to checking const-ness of
expressions.

* Another (possibly different) FE-specific fold should deal with
lowering FE specific trees to pure GENERIC.

* A more powerful fold should be available for more precise warnings
from the FE in some specific cases (it is not clear to me if the
desired behavior is to move these warnings and the folding to
gimplification, or to keep this folding working on trees? Also,
whether the result of this folding should be discarded or stored
somewhere).

* I think a result of the above is that fold in the FEs (for
const-ness, warnings and lowering) should be different from fold in
the middle-end (for optimization), but I am not sure if this is part
of the plan.

Cheers,

Manuel.

Re: Target dependence of conditional expression gimplification

[Jeff Law]

On 05/05/14 09:05, Florian Weimer wrote:
> On 03/03/2014 01:03 PM, Florian Weimer wrote:
>> I have code like this:
>>
>>       if (auth_info->attrs.pin.flags & SC_PKCS15_PIN_FLAG_SO_PIN) {
>>          preferred = 1;
>>      } else {
>>          preferred = current;
>>          /* PINs are even numbered, PUKs are odd */
>>          if (!(preferred & 1))
>>              preferred++;
>>          if (preferred >= 126)
>>              return SC_ERROR_TOO_MANY_OBJECTS;
>>      }
>>
>>      if (current > preferred || preferred > CARDOS_PIN_ID_MAX)
>>          return SC_ERROR_TOO_MANY_OBJECTS;
>>
>> On x86_64, the condition in the final if statement is gimplified as:
>>
>> -  D.8003 = current > preferred;
>> -  D.8004 = preferred > 15;
>> -  D.8005 = D.8003 | D.8004;
>> -  if (D.8005 != 0) goto <D.8006>; else goto <D.8007>;
>>
>> On rs6000 (-mtune=power7 -mcpu=power7), I get this instead:
>>
>> +  if (current > preferred) goto <D.8376>; else goto <D.8378>;
>> +  <D.8378>:
>> +  if (preferred > 15) goto <D.8376>; else goto <D.8377>;
Yes, this is a known issue.  There's places where the costing models 
change the gimple we initially generate and what transformations we 
apply later.

Long term we want to push this stuff to a later point in the pipeline, 
but there's some disagreement over exactly how/when to do that.

Jfef

Re: Target dependence of conditional expression gimplification

[Florian Weimer]

On 05/05/2014 06:11 PM, Jeff Law wrote:

> Yes, this is a known issue.  There's places where the costing models
> change the gimple we initially generate and what transformations we
> apply later.

Well, "known issue" is very subjective.  Our developers run into this 
quite regularly because you get more -Wstrict-overflow warnings on ppc64 
and s390x than on x86_64.  The GIMPLE on x86_64 contains fewer 
conditional statements, so the warning does not fire.

> Long term we want to push this stuff to a later point in the pipeline,
> but there's some disagreement over exactly how/when to do that.

Would it be feasible (for someone like me who is not really familiar 
with GCC internals) to move the BRANCH_COST-dependent bits of fold to an 
early GIMPLE pass?  There's another target dependence related to 
function pointers which could receive similar treatment.

This wouldn't help with the -Wstrict-overflow issue that prompted my 
original message, but it would benefit warnings and other analyses 
performed on early not-fully-optimized GIMPLE.

-- 
Florian Weimer / Red Hat Product Security Team

Re: Target dependence of conditional expression gimplification

[Jeff Law]

On 05/06/14 02:24, Florian Weimer wrote:
> On 05/05/2014 06:11 PM, Jeff Law wrote:
>
>> Yes, this is a known issue.  There's places where the costing models
>> change the gimple we initially generate and what transformations we
>> apply later.
>
> Well, "known issue" is very subjective.  Our developers run into this
> quite regularly because you get more -Wstrict-overflow warnings on ppc64
> and s390x than on x86_64.  The GIMPLE on x86_64 contains fewer
> conditional statements, so the warning does not fire.
That unfortunately happens.  We don't guarantee warning stability across 
platforms.


>
>> Long term we want to push this stuff to a later point in the pipeline,
>> but there's some disagreement over exactly how/when to do that.
>
> Would it be feasible (for someone like me who is not really familiar
> with GCC internals) to move the BRANCH_COST-dependent bits of fold to an
> early GIMPLE pass?  There's another target dependence related to
> function pointers which could receive similar treatment.
Moving the bits is easy.  Verifying that doing so doesn't hurt 
performance is the real work here.  And in the process of looking at the 
performance side, you're going to run into a rats nest of issues.

If it were easy, it'd already have been done.

Jeff