<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Thu, Apr 19, 2018 at 2:53 PM Tim Northover <<a href="mailto:t.p.northover@gmail.com" target="_blank">t.p.northover@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 19 April 2018 at 22:36, Manoj Gupta via llvm-dev<br>
<<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br>
> I was looking around for the cases where AddrSpace !=0 are checked. Seems<br>
> like there are a bunch of optimizations that will fail to apply for non zero<br>
> address spaces.<br>
<br>
Isn't that exactly what we want? Did you look in enough detail to<br>
determine that these optimizations *should* have applied? I'd be<br>
pretty interested to know what we disable for the others, because the<br>
the null thing is the only guarantee I knew about that we made.<br>
<br>
Also, I'm pretty unconvinced optimization should be our first priority<br>
here. That smacks of the untenable (IMO) option 2. Whatever we do I<br>
want a program with well-defined semantics at the end. Starting from a<br>
known good position and enabling desirable optimizations that we have<br>
decided are valid is a significantly better path than starting with<br>
"you might get lucky" and disabling extra optimizations that are<br>
reported to us.<br>
<br>
I realise this actually argues against my datalayout suggestion too,<br>
so I'm getting a lot more convinced by Eli (& Duncan).<br>
<br>
Cheers.<br>
<br>
Tim.<br></blockquote><div><br></div><div>Thanks a lot Tim,</div><div><br></div><div>My understanding is we only want to disable the optimizations regarding undefined behavior</div><div>related to null pointer deference optimizations. And address space checks will end up</div><div>disabling more optimizations than needed.</div><div>I did look at some of the optimizations/transforms and there are some that we definitely want to keep.</div><div><br></div><div>Just a quick example from grepping: lib/Transforms/Scalar/LoopIdiomRecognize.cpp</div><div><div>...........</div><div> // Don't create memset_pattern16s with address spaces.</div><div> StorePtr->getType()->getPointerAddressSpace() == 0 &&</div><div> (PatternValue = getMemSetPatternValue(StoredVal, DL))) {</div><div> // It looks like we can use PatternValue!</div><div> return LegalStoreKind::MemsetPattern;</div><div> }</div></div><div><br></div><div>Even worse, Sanitizers do NOT work with address spaces which is a big deal breaker IMO.</div><div><br></div><div><span style="color:rgb(34,34,34);font-family:sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">Since address spaces and null pointers are really orthogonal issues, I would prefer to</span></div><div><span style="color:rgb(34,34,34);font-family:sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">not conflate them.</span><br></div><div><span style="color:rgb(34,34,34);font-family:sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><br></span></div><div>In addition, It is already not easy to convince Linux Kernel maintainers to accept clang specific patches.</div><div>Worse performance when compared to GCC may make it even harder to push more patches.</div><div>(There are already many complains about clang not supporting optimizations that Linux kernel is used to.</div><div><div style="color:rgb(34,34,34);font-family:sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration-style:initial;text-decoration-color:initial;background-color:rgb(255,255,255)">As a side note: x86 maintainers deliberately broke clang support in upstream (<a href="https://lkml.org/lkml/2018/4/2/115" style="color:rgb(17,85,204)" target="_blank">https://lkml.org/lkml/2018/4/2/115</a>)</div><div style="color:rgb(34,34,34);font-family:sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration-style:initial;text-decoration-color:initial;background-color:rgb(255,255,255)">related to asm-goto support. It would be greatly preferable to avoid another confrontation related to performance).</div></div><div><br></div><div><span style="color:rgb(34,34,34);font-family:sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="color:rgb(34,34,34);font-family:sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">> Starting from a </span><span style="color:rgb(34,34,34);font-family:sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">known good position and enabling desirable optimizations that we have</span><br style="color:rgb(34,34,34);font-family:sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial"><span style="color:rgb(34,34,34);font-family:sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">> decided are valid is a significantly better path than starting with </span><span style="color:rgb(34,34,34);font-family:sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">"you might get lucky"</span></span></div><div><span style="color:rgb(34,34,34);font-family:sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="color:rgb(34,34,34);font-family:sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">> and disabling extra optimizations that are </span><span style="color:rgb(34,34,34);font-family:sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">reported to us.</span><br></span></div><div><span style="color:rgb(34,34,34);font-family:sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><br></span></div><div><span style="color:rgb(34,34,34);font-family:sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">From Linux kernel maintainers POV, Clang built kernels are already "getting lucky". So I am not too</span></div><div><span style="color:rgb(34,34,34);font-family:sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">worried about missing a few cases.</span></div><div>I'll be glad to fix the missing cases whenever reported.</div><div><br></div><div>I also have some other concerns with address spaces e.g. how to pick a safe address space not used by</div><div> any target e.g. many targets (in and out-of-tree) actively use non-zero address spaces.<br></div><div>User code can also specify any address space via __attribute__((address_space(N))) so mixing object</div><div>files will be tricky in such cases.</div><div><br></div><div><br></div><div>I hope that I have made the case for not using address spaces. Not that I am against address spaces but</div><div>losing performance and sanitizers support is a deal breaker.</div><div><br></div><div>Thanks</div><div>-Manoj</div></div></div>