llvm dev - Oct 2019 - Full restrict support - status update

Hi all,

## Status:

During the past weeks I have updated the restrict patches with various
improvements:
- the ScopedNoAliasAA now also works together with the new pass manager
- the SLPVectorizer now works nice with the noalias support.
- there were some issues with some of the options enabling/disabling full
restrict.  These have been fixed.
- various smaller enhancements.

Today, I rebased the patches. [1]
Based on the feedback at the 'full restrict' roundtable, I also created
a single patch containing all changes. [2]

## Request for testing and feedback:

Extra help with the code review would be great [1].
This includes feedback on the design decisions and naming.

It would also be great if you could just try it out and check the effect on your
benchmarks and testcases.
The single patch[2] should make it more convenient to try it out:
- If the full restrict support triggers a problem, I would like to hear about
it.
- But, if it works and improves your benchmarks, I also would like to hear about
it, either through
  phabricator, the llvm mailing list or in private. 

## Known issues:

- For now, there is still no llvm-ir bitcode support for the load/store
noalias_sidechannel argument.
- the
'SingleSource/Regression/C/gcc-c-torture/execute/GCC-C-execute-pr38212.test'
in the 'test-suite' fails:
-- the test is wrong as it triggers undefined behavior: it reads and writes the
same object through 2 different
    restrict pointers which have been declared in the same scope.

## Future changes
- Another request that came up during the round table, is to split up the
documentation in two parts:
   a separate document describing the noalias architecture, and the LangRef,
describing the intrinsics.
  I'll be working on that in the coming days.

Thanks,

Jeroen Dobbelaere

[1] https://reviews.llvm.org/D68484  [PATCH 01/38] [noalias] LangRef: noalias
intrinsics and noalias_sidechannel documentation.
[2] https://reviews.llvm.org/D69542 Full Restrict Support - single patch

On 10/28/19 8:12 PM, Jeroen Dobbelaere via llvm-dev
wrote:
> Hi all,
>
> ## Status:
>
> During the past weeks I have updated the restrict patches with various
improvements:
> - the ScopedNoAliasAA now also works together with the new pass manager
> - the SLPVectorizer now works nice with the noalias support.
> - there were some issues with some of the options enabling/disabling full
restrict.  These have been fixed.
> - various smaller enhancements.
>
> Today, I rebased the patches. [1]
> Based on the feedback at the 'full restrict' roundtable, I also
created a single patch containing all changes. [2]
>
> ## Request for testing and feedback:
>
> Extra help with the code review would be great [1].
> This includes feedback on the design decisions and naming.
>
> It would also be great if you could just try it out and check the effect on
your benchmarks and testcases.
> The single patch[2] should make it more convenient to try it out:
> - If the full restrict support triggers a problem, I would like to hear
about it.
> - But, if it works and improves your benchmarks, I also would like to hear
about it, either through
>    phabricator, the llvm mailing list or in private.
>
> ## Known issues:
>
> - For now, there is still no llvm-ir bitcode support for the load/store
noalias_sidechannel argument.
> - the
'SingleSource/Regression/C/gcc-c-torture/execute/GCC-C-execute-pr38212.test'
in the 'test-suite' fails:
> -- the test is wrong as it triggers undefined behavior: it reads and writes
the same object through 2 different
>      restrict pointers which have been declared in the same scope.

This, we can fix now independent of anything else. We should either fix 
the test, if there's a reasonable way to fix it, or we should remove it. 
If you can post a patch with an explanation of the problem, that would 
be great.

Thanks again,

Hal

>
> ## Future changes
> - Another request that came up during the round table, is to split up the
documentation in two parts:
>     a separate document describing the noalias architecture, and the
LangRef, describing the intrinsics.
>    I'll be working on that in the coming days.
>
> Thanks,
>
> Jeroen Dobbelaere
>
> [1] https://reviews.llvm.org/D68484  [PATCH 01/38] [noalias] LangRef:
noalias intrinsics and noalias_sidechannel documentation.
> [2] https://reviews.llvm.org/D69542 Full Restrict Support - single patch
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory

> On 29 Oct 2019, at 4:06 am, Finkel, Hal J. via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> On 10/28/19 8:12 PM, Jeroen Dobbelaere via llvm-dev wrote:
>> 
>> - the
'SingleSource/Regression/C/gcc-c-torture/execute/GCC-C-execute-pr38212.test'
in the 'test-suite' fails:
>> -- the test is wrong as it triggers undefined behavior: it reads and
writes the same object through 2 different
>>     restrict pointers which have been declared in the same scope.
> 
> 
> This, we can fix now independent of anything else. We should either fix 
> the test, if there's a reasonable way to fix it, or we should remove
it.
> If you can post a patch with an explanation of the problem, that would 
> be great.
I’m wary of updating the test itself, as someone may re-vendor the GCC C torture
suite, undoing those edits. However, we already exclude some tests in the GCC
torture suite because they rely on UB. Having inspected pr38212.c, I am happy to
add this test to the list of excluded tests with the explanation provided. I’ll
commit that change today.

Sam

--
Sam Elliott
Software Developer - LLVM
lowRISC CIC
--

Jeroen Dobbelaere via llvm-dev <llvm-dev at lists.llvm.org> writes:
> - the
'SingleSource/Regression/C/gcc-c-torture/execute/GCC-C-execute-pr38212.test'
in the 'test-suite' fails:
> -- the test is wrong as it triggers undefined behavior: it reads and writes
the same object through 2 different
>     restrict pointers which have been declared in the same scope.
What's the failure mode?  Wrong answers or compiler abort?  If the
latter, it would be nice if LLVM could emit a warning about illegal use
of restrict.  Longer term, a RestrictSanitizer would be really helpful.

                      -David

On Tue, Oct 29, 2019 at 9:26 PM David Greene via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
>
> Jeroen Dobbelaere via llvm-dev <llvm-dev at lists.llvm.org> writes:
>
> > - the
'SingleSource/Regression/C/gcc-c-torture/execute/GCC-C-execute-pr38212.test'
in the 'test-suite' fails:
> > -- the test is wrong as it triggers undefined behavior: it reads and
writes the same object through 2 different
> >     restrict pointers which have been declared in the same scope.
>
> What's the failure mode?  Wrong answers or compiler abort?  If the
> latter,
> it would be nice if LLVM could emit a warning about illegal use
> of restrict.  Longer term, a RestrictSanitizer would be really helpful.
Yes :)
I would think it would not require any runtime (think - asan/msan) support,
so i'm not sure why it could not be a part of UBSan proper.
In other words i'd like to *tentatively* claim this,
i may be interested to look into it, after the restrict support lands :)
>                       -David
Roman
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

> -----Original Message-----
> From: David Greene <greened at obbligato.org>
[...]
> What's the failure mode?  Wrong answers or compiler abort?  If the
> latter, it would be nice if LLVM could emit a warning about illegal use
> of restrict.  Longer term, a RestrictSanitizer would be really helpful.
> 
>                       -David
The compiler produces code that does not do what the testcase expects it to do.
This particular case could be detected and warned for at compile time (after
inlining and constant propagation).

Greetings,

Jeroen

Hi Jeroen,

Thank you very much for the great work, it is much appreciated.
> - For now, there is still no llvm-ir bitcode support for the load/store
noalias_sidechannel argument.

Do you have plans to work on this in the near future? Do you know how much
work it is and if there are significant hurdles?

Thanks,
Alexey

On Mon., Oct. 28, 2019, 9:12 p.m. Jeroen Dobbelaere via llvm-dev, <
llvm-dev at lists.llvm.org> wrote:
> Hi all,
>
> ## Status:
>
> During the past weeks I have updated the restrict patches with various
> improvements:
> - the ScopedNoAliasAA now also works together with the new pass manager
> - the SLPVectorizer now works nice with the noalias support.
> - there were some issues with some of the options enabling/disabling full
> restrict.  These have been fixed.
> - various smaller enhancements.
>
> Today, I rebased the patches. [1]
> Based on the feedback at the 'full restrict' roundtable, I also
created a
> single patch containing all changes. [2]
>
> ## Request for testing and feedback:
>
> Extra help with the code review would be great [1].
> This includes feedback on the design decisions and naming.
>
> It would also be great if you could just try it out and check the effect
> on your benchmarks and testcases.
> The single patch[2] should make it more convenient to try it out:
> - If the full restrict support triggers a problem, I would like to hear
> about it.
> - But, if it works and improves your benchmarks, I also would like to hear
> about it, either through
>   phabricator, the llvm mailing list or in private.
>
> ## Known issues:
>
> - For now, there is still no llvm-ir bitcode support for the load/store
> noalias_sidechannel argument.
> - the
>
'SingleSource/Regression/C/gcc-c-torture/execute/GCC-C-execute-pr38212.test'
> in the 'test-suite' fails:
> -- the test is wrong as it triggers undefined behavior: it reads and
> writes the same object through 2 different
>     restrict pointers which have been declared in the same scope.
>
> ## Future changes
> - Another request that came up during the round table, is to split up the
> documentation in two parts:
>    a separate document describing the noalias architecture, and the
> LangRef, describing the intrinsics.
>   I'll be working on that in the coming days.
>
> Thanks,
>
> Jeroen Dobbelaere
>
> [1] https://reviews.llvm.org/D68484  [PATCH 01/38] [noalias] LangRef:
> noalias intrinsics and noalias_sidechannel documentation.
> [2] https://reviews.llvm.org/D69542 Full Restrict Support - single patch
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://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/20191031/2a5bbd68/attachment-0001.html>

Hi Alexey,

Adding llvm-ir bitcode support means adding/adapting the tags for LOAD/STORE
instructions and adding
the support for the noalias_sidechannel at the right places.

I had a short attempt to implement it when preparing the public patches, but I
am not familiar with that
part of the llvm code. When I noticed that it would take a lot longer than
anticipated, I postponed it.
Also because it is likely that the way how the noalias_sidechannel was added to
LoadInst/StoreInst might
change.

At this moment, I am not planning to work on this. For the current
implementation, there might be
a number of possibilities for adding support :
  - maybe 2 new tags are needed (FUNC_CODE_INST_{LOAD_NOALIAS,STORE_NOALIAS})
  - or maybe it is sufficient to add the noalias_sidechannels as extra operands
and look at the number of
     operands to see if they are present or not
  - or maybe it is sufficient to look at the number of operands, and the
noalias_sidechannel operand should
    be added with an extra bit, indicating if it is really there or not...

Greetings,

Jeroen Dobbelaere


From: Alexey Zhikhartsev <alexey.zhikhar at gmail.com>
Sent: Thursday, October 31, 2019 16:21
To: Jeroen Dobbelaere <dobbel at synopsys.com>
Cc: llvm-dev at lists.llvm.org
Subject: Re: [llvm-dev] Full restrict support - status update

Hi Jeroen,

Thank you very much for the great work, it is much appreciated.
> - For now, there is still no llvm-ir bitcode support for the load/store
noalias_sidechannel argument.
Do you have plans to work on this in the near future? Do you know how much work
it is and if there are significant hurdles?

Thanks,
Alexey

[...]
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20191103/9bf0ca32/attachment.html>