<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">Just to chime in that we have been seeing a bunch of instances on aarch64 as well where the load widening prevented LICM and probably worse inserted additional shifts instructions into the critical path. This was showing especially after r246985 (see for example <a href="http://llvm.org/PR25321" class="">http://llvm.org/PR25321</a>) which leads to additional widening possibilities.</div><div class=""><br class=""></div><div class="">So I'd also strongly suggest to only optimize obvious cases and stay away from variations that lead to additional shift instructions being inserted.</div><div class=""><br class=""></div><div class="">(I missed D24096, but will check whether that fixes our performance problems as well).</div><div class=""><br class=""></div><div class="">- Matthias</div><br class=""><div><blockquote type="cite" class=""><div class="">On Sep 28, 2016, at 5:25 PM, Sanjoy Das via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" class="">llvm-dev@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="">Hi David,<br class=""><br class="">David Chisnall via llvm-dev wrote:<br class="">> On 28 Sep 2016, at 16:50, Philip Reames via llvm-dev<<a href="mailto:llvm-dev@lists.llvm.org" class="">llvm-dev@lists.llvm.org</a>> wrote:<br class="">>> At this point, my general view is that widening transformations of any kind should be done very late. Ideally, this is something the backend would do, but doing it as a CGP like fixup pass over the IR is also reasonable.<br class="">><br class="">> I’m really glad to see that this is gone in GVN - it will reduce our<br class="">> diffs a lot when we do the next import. The GVN load widening is not<br class="">> sound in the presence of hardware-enforced spacial memory safety, so<br class="">> we ended up with the compiler inserting things that caused hardware<br class="">> bounds checks to fail and had to disable it in a few places.<br class="">><br class="">> If you’re reintroducing it, please can we have a backend option to<br class="">> specify whether it’s valid to widen loads beyond the extents (for<br class="">> example, for us it is not valid to widen an i16 load followed by an i8<br class="">> load to an i32 load). Ideally, we’d also want the back end to supply<br class=""><br class="">Don't you have to mark the functions you generate as<br class="">"Attribute::SanitizeAddress"? We should definitely make the<br class="">speculative form of this transform (i.e. "load i32, load i16" --><br class="">"load i64") predicated on Attribute::SanitizeAddress.<br class=""><br class="">-- Sanjoy<br class="">_______________________________________________<br class="">LLVM Developers mailing list<br class=""><a href="mailto:llvm-dev@lists.llvm.org" class="">llvm-dev@lists.llvm.org</a><br class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev<br class=""></div></div></blockquote></div><br class=""></body></html>