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

[Manoj Gupta via cfe-dev]

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/cfe-dev/attachments/20180419/56613e48/attachment.html>