Bug 91191 - vrp and boolean arguments
Summary: vrp and boolean arguments
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 9.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks: VRP
  Show dependency treegraph
 
Reported: 2019-07-18 01:11 UTC by Jim Wilson
Modified: 2021-03-02 22:26 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2019-07-18 00:00:00


Attachments
untested patch (1.54 KB, patch)
2019-07-18 07:38 UTC, Richard Biener
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jim Wilson 2019-07-18 01:11:22 UTC
It appears that vrp isn't propagating the ranges of incoming boolean arguments.  Given this example:

unsigned char reg(_Bool b) {
    union U {
        unsigned char f0;
        _Bool f1;
    };
    union U u;
    u.f1 = b;
    if (u.f0 > 1) { 
        // This cannot happen
        // if b is only allowed
        // to be 0 or 1:
        return 42;   
    }
    return 13;
}

clang optimizes this to unconditionally return 13, but gcc does a compare and conditionally returns either 42 or 13 depending on the result of the compare.
This happens with both x86_64 and RISC-V.

Looking at the vrp dumps, I see
b_3(D): VARYING
Comment 1 Richard Biener 2019-07-18 07:37:52 UTC
The issue is that VRP doesn't handle the way we optimize the union punning:

  <bb 2> :
  _6 = VIEW_CONVERT_EXPR<unsigned char>(b_4(D));
  if (_6 > 1)

if we decide to rely on appropriate sign/zero-extending of sub-size precision
entities (otherwise we'd have simplified the VIEW_CONVERT to a simple
conversion) the attached fixes the missed optimzation.
Comment 2 Richard Biener 2019-07-18 07:38:16 UTC
Created attachment 46610 [details]
untested patch
Comment 3 Richard Biener 2019-07-31 08:28:56 UTC
Andrew is now maintaining VRP.
Comment 4 Andrew Macleod 2020-11-17 23:47:03 UTC
Its unclear to me what should be done here.

the documentation for VIEW_CONVERT is quite clear in tree.def:
"The only operand is the value to be viewed as being of another type. It is undefined if the type of the input and of the expression have different sizes."

I realized range-ops isn't currently doing anything with VIEW_CONVERT, but when I tried changing this test case to similar precisions, the VIEW_CONVERT goes away.  My knowledge of VIEW_CONVERT is, well, poor to non-existent.

so
1) Why are we issuing a VIEW_CONVERT in the first place?  They are different precision's and this appears to break the very definition

2) when they are the same precision, there wouldn't need to be a sign extension, so how does it vary from a cast?

1) and 2) seem incongruent to me.  Either you confirm they are the same size and do a cast, or they are different precisions and you want to avoid the normal cast behaviour.

Or has this got something to do with a difference in precision but understanding the the underlying storage is actually the same size?  So we only issue a VIEW_CONVERT when precision is different but storage is the same size? so you can make certain assumption about the value?   but that seem wrought with issues too...
Comment 5 Jeffrey A. Law 2020-12-04 19:21:18 UTC
The best way to think about V_C_E is that it's the same bits, just viewed in a different type whereas a cast can do things like sign/zero extension, truncation of floating point values to integers, etc).
Comment 6 Andrew Macleod 2020-12-04 21:26:53 UTC
and when the precision is different.... what?  assume 0's for the missing bits?
If we allow and want that behaviour, we should change the documentation to reflect that...
Comment 7 Jeffrey A. Law 2020-12-05 05:13:31 UTC
If you're V_C_E-ing to a narrower type, you just ignore the bits outside the target type, it's a lot like a narrowing subreg in the RTL world.

I don't know what the semantics are for the widening case.  ISTM that it's not really helpful there.

At least that's my understanding after discussing pr80635 with Richi.
Comment 8 Andrew Macleod 2021-02-19 17:00:31 UTC
(In reply to Jeffrey A. Law from comment #7)
> If you're V_C_E-ing to a narrower type, you just ignore the bits outside the
> target type, it's a lot like a narrowing subreg in the RTL world.
> 
> I don't know what the semantics are for the widening case.  ISTM that it's
> not really helpful there.
> 

But isn't that the case here?

    _6 = VIEW_CONVERT_EXPR<unsigned char>(b_4(D));
    if (_6 > 1)
      goto <bb 3>; [INV]
    else
      goto <bb 4>; [INV]

we are widening the single bit BOOL into an 8 bit unsigned char...
so we need to decide if the other bits are always 0 or what....
Comment 9 Jeffrey A. Law 2021-03-02 22:26:08 UTC
Yea, I think so -- which would be undefined because the sizes are different according to the docs you referenced.  Which would also invalidate part of my responses in c#5 and c#7.

It sounds like something ought to be checking the constraint that the input and output must have the same size.