[PATCH] D26718: [llvm] Iterate SmallPtrSet in reverse order to uncover non-determinism in codegen

[Mehdi Amini via llvm-commits]

> On Dec 7, 2016, at 1:11 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> 
>> 
>> On 2016-Dec-07, at 12:15, Mehdi AMINI via Phabricator <reviews at reviews.llvm.org> wrote:
>> 
>> mehdi_amini added inline comments.
>> 
>> 
>> ================
>> Comment at: include/llvm/ADT/SmallPtrSet.h:30
>> +#if LLVM_ENABLE_ABI_BREAKING_CHECKS
>> +extern bool ReverseIterate;
>> +#endif
>> ----------------
>> mgrang wrote:
>>> mehdi_amini wrote:
>>>> mgrang wrote:
>>>>> mgrang wrote:
>>>>>> mehdi_amini wrote:
>>>>>>> This introduces a dependency from ADT to Support, I'm not sure it is OK.
>>>>>>> Also the symbol should be in the LLVM namespace.
>>>>>> @mehdi_amini Yes, I will move ReverseIterate to LLVM namespace.
>>>>>> 
>>>>>> Initially I was skeptical about defining ReverseIterate in Support. But then I did not know of a better place to do this, Could you please suggest what would be a preferable place to define this?
>>>>> @mehdi_amini Could you please take a look at my updated patch and comment when you get a chance?
>>>> I don't have a good suggestion. This is quite annoying. Maybe making them weak definition in a header (possibly using template)?
>>> @mehdi_amini Thanks for the suggestion.
>>> 
>>> Variable templates are a C++14 feature and the compiler will output this warning:
>>> "warning: variable templates are a C++14 extension [-Wc++14-extensions]"
>>> 
>>> Would we need to silence these? By passing -Wno-c++14-extensions in the CMake flags?
>> No this is not OK, LLVM builds in C++11 mode.
>> I didn't expect you to need a variable template, but a static member in a templated class.
> 
> Something like this should work:
> --
> template <class T = void> struct ReverseIterateImpl { static bool value = ...; };

Initialization has to be out-of-line I believe:

template <class T = void> struct ReverseIterateImpl { static bool value; };
template <class T> 
bool ReverseIterateImpl<T>::value = false;


> 
> typedef ReverseIterateImpl<> ReverseIterate;
> --
> then you can use it like this:
> --
> if (ReverseIterate::value) { ... }
> --
> 
>> 
>> 
>> https://reviews.llvm.org/D26718 <https://reviews.llvm.org/D26718>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161207/058e3529/attachment.html>