Re: [PATCH] middle-end IFN_ASSUME support [PR106654]

[Martin Uecker <ma.uecker@gmail.com>]

Am Freitag, den 14.10.2022, 23:20 +0200 schrieb Jakub Jelinek:
> On Fri, Oct 14, 2022 at 10:43:16PM +0200, Martin Uecker wrote:
> > Am Montag, den 10.10.2022, 10:54 +0200 schrieb Jakub Jelinek:
> > > Hi!
> > > 
> > > My earlier patches gimplify the simplest non-side-effects assumptions
> > > into if (cond) ; else __builtin_unreachable (); and throw the rest
> > > on the floor.
> > > The following patch attempts to do something with the rest too.
> > 
> > My recommendation would be to only process side-effect-free
> > assumptions and warn about the rest (clang does this for
> > __builtin_assume).   I do not think this is worth the
> 
> I think that is bad choice and makes it useless.

I think

[[assume(10 == i + 1)]]

could be useful as it is nicer syntax than

if (10 != i + 1)
  unreachable();

 but

[[assume(10 == i++)]]

is confusing / untestable and therefor seems a bit
dangerous. 

But we do not have to agree, I just stating my opinion
here. I would personally then suggest to have an
option for warning about side effects in assume.

> > complexity and I am not so sure the semantics of a
> > hypothetical evaluation are terribly well defined.
> 
> I think the C++23 paper is quite clear.  Yes, you can't verify it
> in debug mode, but there are many other UBs that are hard to verify
> through runtime instrumentation.

And none are good. Some are very useful though
(or even required in the context of C/C++). But I think
there should be a very good reason for adding more.

Personally, I do not see [[assume]] how side effects
is useful enough to justify adding another kind of
untestable UB.

Especially also because it has somewhat vaguely 
defined semantics. I don't know any formalization the
proposed semantics and the normative wording
"the converted expression would evaluate to true"
is probably good starting point for a PhD thesis.


> And, OpenMP has a similar feature (though, in that case it is even
> a stronger guarantee where something is guaranteed to hold across
> a whole region rather than just on its entry.
>
> > That you can not verify this properly by turning it
> > into traps in debug mode (as this would execute the
> > side effects) also makes this a terrible feature IMHO.
> > 
> > MSVC which this feature was based does not seem to make
> > much to sense to me: https://godbolt.org/z/4Ebar3G9b
> 
> So maybe their feature is different from what is in C++23,
> or is badly implemented?

The paper says as the first sentence in the abstract:

"We propose a standard facility providing the semantics
of existing compiler built-ins such as
__builtin_assume (Clang) and __assume (MSVC, ICC)."

But Clang does not support side effects and MSVC
is broken.  But yes, the paper then describes these
extended semantics for expression with side effects. 
According to the author this was based on discussions
with MSVC developers, but - to me - the fact that MSVC
still implements something else which does not seem
to make sense speaks against this feature.


> I think with what we have in the works for GCC we'll be able to optimize
> in
> int f(int i)
> {
>   [[assume(1 == i++)]];
>   return (1 == i++);
> }
> 
> int g(int i)
> {
>   [[assume(1 == ++i)]];
>   return (1 == ++i);
> }
> 
> extern int i;
> 
> int h(void) 
> {
>   [[assume(1 == ++i)]];
>   return (1 == ++i);
> }
> 
> 
> int k(int i)
> {
>   [[assume(42 == ++i)]];
>   return i;
> }
> at least f/g to return 1; and k to return 41;
> The h case might take a while to take advantage of.

But why? Do we really want to encourage people to
write such code?


Martin