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

[Mandeep Singh Grang via Phabricator via llvm-commits]

mgrang added inline comments.


================
Comment at: include/llvm/ADT/SmallPtrSet.h:30
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
+extern bool ReverseIterate;
+#endif
----------------
mehdi_amini wrote:
> 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.
Ah got it. Thanks! Will push a patch.


https://reviews.llvm.org/D26718