llvm dev - Apr 2018 - [cfe-dev] RFC: Implementing -fno-delete-null-pointer-checks in clang

On Thu, Apr 19, 2018 at 11:59 AM Friedman, Eli <efriedma at
codeaurora.org>
wrote:
> On 4/19/2018 11:57 AM, Friedman, Eli via cfe-dev wrote:
>
> On 4/19/2018 11:48 AM, Manoj Gupta via llvm-dev wrote:
>
> On Wed, Apr 18, 2018 at 12:54 PM Tim Northover <t.p.northover at
gmail.com>
> wrote:
>
>
> On Wed, Apr 18, 2018 at 12:02 PM Friedman, Eli <efriedma at
codeaurora.org>
>> wrote:
>
> > Despite the name, the flag actually has rather straightforward
semantics
>> > from the compiler's perspective.  From the gcc docs for
>> > -fdelete-null-pointer-checks: "Assume that programs cannot
safely
>> > dereference null pointers, and that no code or data element
resides at
>> > address zero."  (-fno-delete-null-pointer-checks is the
opposite.)
>>
>> Ah, now that's quite a bit more palatable. I really should have
read
>> the docs before spouting "my favourite rant #1". Then my main
concern
>> is that we end up with a sufficiently clear (and documented)
>> definition that we're not promising more than we intend. I get very
>> grumpy if I can't tell someone with UB that they're Doing It
Wrong.
>>
>> Of the two options, I still think the second is a non-starter.
>> Something between the two might be a datalayout flag specifying
>> addrspace(0) behaviour. It's pretty easy to argue that it'd be
good if
>> code used some kind of
>> "DataLayout::isPointerIntrinsicallyInvalid(Value *)" for this
kind of
>> thing anyway (rename or relocate at will).
>>
>> And the name really is terrible, we should change it if at all feasible
>
>
>
> On Wed, Apr 18, 2018 at 1:46 PM Jon Chesterfield via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> I'm working with an embedded architecture that could, in some
situations,
>> run faster if code or data could be located at address zero. I
don't know
>> whether this applies to other embedded chips.
>>
>> Despite the name, the flag actually has rather straightforward
semantics
>>> from the compiler's perspective.  From the gcc docs for
>>> -fdelete-null-pointer-checks: "Assume that programs cannot
safely
>>> dereference null pointers, and that no code or data element resides
at
>>> address zero."  (-fno-delete-null-pointer-checks is the
opposite.)
>>>
>>> -Eli
>>>
>>>
>  Thanks Tim and Eli,
> I should have put the GCC description for the flag in the email.
>
> Regarding the suggestion to DataLayout, It is an interesting idea. I like
> it in particular since it will avoid creating a new llvm optimization flag
> and is auto embedded in IR.
>
> Given that, how do we want to proceed, do we want to add yet another field
> to the DataLayout string?
>
>
> Modifying the datalayout is not a good idea; it doesn't interact with
LTO
> correctly, and the frontend and the backend generally need to agree on the
> datalayout.
>
> You could maybe use a module flag, if the address-space thing doesn't
work
> for some reason, but we don't like to introduce more of those if we can
> avoid it.
>
>
> Actually, thinking about it a bit more, a function attribute would be
> better than a module flag.  But I'd still like to explore the
address-space
> thing first, since we already have the LLVM infrastructure to make that
> work.
>
> -Eli
>
Thanks Eli,

I was looking around for the cases where AddrSpace !=0 are checked. Seems
like there are a bunch of optimizations that will fail to apply for non
zero address spaces.
So I'll start with the function attribute approach.

-Manoj
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180419/f4105d91/attachment.html>

On 19 April 2018 at 22:36, Manoj Gupta via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> I was looking around for the cases where AddrSpace !=0 are checked. Seems
> like there are a bunch of optimizations that will fail to apply for non
zero
> address spaces.
Isn't that exactly what we want? Did you look in enough detail to
determine that these optimizations *should* have applied? I'd be
pretty interested to know what we disable for the others, because the
the null thing is the only guarantee I knew about that we made.

Also, I'm pretty unconvinced optimization should be our first priority
here. That smacks of the untenable (IMO) option 2. Whatever we do I
want a program with well-defined semantics at the end. Starting from a
known good position and enabling desirable optimizations that we have
decided are valid is a significantly better path than starting with
"you might get lucky" and disabling extra optimizations that are
reported to us.

I realise this actually argues against my datalayout suggestion too,
so I'm getting a lot more convinced by Eli (& Duncan).

Cheers.

Tim.

On Thu, Apr 19, 2018 at 2:53 PM Tim Northover <t.p.northover at gmail.com>
wrote:
> On 19 April 2018 at 22:36, Manoj Gupta via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
> > I was looking around for the cases where AddrSpace !=0 are checked.
Seems
> > like there are a bunch of optimizations that will fail to apply for
non
> zero
> > address spaces.
>
> Isn't that exactly what we want? Did you look in enough detail to
> determine that these optimizations *should* have applied? I'd be
> pretty interested to know what we disable for the others, because the
> the null thing is the only guarantee I knew about that we made.
>
> Also, I'm pretty unconvinced optimization should be our first priority
> here. That smacks of the untenable (IMO) option 2. Whatever we do I
> want a program with well-defined semantics at the end. Starting from a
> known good position and enabling desirable optimizations that we have
> decided are valid is a significantly better path than starting with
> "you might get lucky" and disabling extra optimizations that are
> reported to us.
>
> I realise this actually argues against my datalayout suggestion too,
> so I'm getting a lot more convinced by Eli (& Duncan).
>
> Cheers.
>
> Tim.
>
Thanks a lot Tim,

My understanding is we only want to disable  the optimizations regarding
undefined behavior
related to null pointer deference optimizations. And address space checks
will end up
disabling more optimizations than needed.
I did look at some of the optimizations/transforms and there are some that
we definitely want to keep.

Just a quick example from grepping:
lib/Transforms/Scalar/LoopIdiomRecognize.cpp
...........
             // Don't create memset_pattern16s with address spaces.
             StorePtr->getType()->getPointerAddressSpace() == 0 &&
             (PatternValue = getMemSetPatternValue(StoredVal, DL))) {
    // It looks like we can use PatternValue!
    return LegalStoreKind::MemsetPattern;
  }

Even worse, Sanitizers do NOT work with address spaces which is a big deal
breaker IMO.

Since address spaces and null pointers are really orthogonal issues, I
would prefer to
not conflate them.

In addition, It is already not easy to convince Linux Kernel maintainers to
accept clang specific patches.
Worse performance when compared to GCC may make it even harder to push more
patches.
(There are already many complains about clang not supporting optimizations
that Linux kernel is used to.
As a side note: x86 maintainers deliberately broke clang support in
upstream (https://lkml.org/lkml/2018/4/2/115)
related to asm-goto support. It would be greatly preferable to avoid
another confrontation related to performance).
> Starting from a known good position and enabling desirable optimizations
that we have
> decided are valid is a significantly better path than starting with
"you
might get lucky"
> and disabling extra optimizations that are reported to us.
>From Linux kernel maintainers POV, Clang built kernels are already
"getting
lucky". So I am not too
worried about missing a few cases.
I'll be glad to fix the missing cases whenever reported.

I also have some other concerns with address spaces e.g. how to pick a safe
address space not used by
 any target e.g. many targets (in and out-of-tree) actively use non-zero
address spaces.
User code can also specify any address space via
__attribute__((address_space(N))) so mixing object
files will be tricky in such cases.


I hope that I have made the case for not using address spaces. Not that I
am against address spaces but
losing performance and sanitizers support is a deal breaker.

Thanks
-Manoj
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180419/56613e48/attachment.html>