<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jun 25, 2015 at 11:46 AM, Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"><br>
> On 2015-Jun-25, at 10:27, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
><br>
><br>
><br>
> On Wed, Jun 24, 2015 at 3:28 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br>
><br>
> > On 2015 Jun 20, at 01:39, Justin Bogner <<a href="mailto:mail@justinbogner.com">mail@justinbogner.com</a>> wrote:<br>
> ><br>
> > David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> writes:<br>
> >> On Fri, Jun 19, 2015 at 11:52 PM, Justin Bogner <<a href="mailto:mail@justinbogner.com">mail@justinbogner.com</a>> wrote:<br>
> >>> David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> writes:<br>
> >>>> On Fri, Jun 19, 2015 at 11:24 PM, Justin Bogner <<a href="mailto:mail@justinbogner.com">mail@justinbogner.com</a> wrote:<br>
> >>>>> -  while (!DeadInsts.empty())<br>
> >>>>> -    if (Instruction *Inst =<br>
> >>>>> -          dyn_cast_or_null<Instruction>(&*DeadInsts.pop_back_val()))<br>
> >>>>> +  while (!DeadInsts.empty()) {<br>
> >>>>> +    Value *V = static_cast<Value *>(DeadInsts.pop_back_val());<br>
> >>>>> +    if (Instruction *Inst = dyn_cast_or_null<Instruction>(V))<br>
> >>>>><br>
> >>>> Hmm - I think there's some fancy machinery in the llvm cast stuff to allow<br>
> >>>> us to map through from different types (so we could say that casting a<br>
> >>>> WeakVH retrieves the Value* first), maybe... (so you could just<br>
> >>>> dyn_cast_or_null<Value*>(DeadInsts.pop_back_val()) directly)<br>
> >>><br>
> >>> I don't really understand what you're going for here - we need to cast<br>
> >>> twice. We have a WeakVH, which we can get a Value* out of, then we<br>
> >>> dyn_cast_or_null that to an Instruction*. There's no need to dyn_cast to<br>
> >>> a Value* (we want the conversion operator), and obviously WeakVH can't<br>
> >>> be cast to Instruction* directly, since they're unrelated types.<br>
> >><br>
> >> What I mean is we have a way of making them related, through a template<br>
> >> called simplify_type. Here's the version for QualType to allow QualTypes to<br>
> >> be casted to specific clang::Types: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__clang.llvm.org_doxygen_&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=J3cwkqKWFm2MzMWCSnVatm74UA_CZNIl2wiRaBk0j44&s=uwRztIRQi8ZVUSUT1J37_HhGoklgbYSJlXHTEFbHlXI&e=" rel="noreferrer" target="_blank">http://clang.llvm.org/doxygen/</a><br>
> >> structllvm_1_1simplify__type_3_01_1_1clang_1_1QualType_01_4.html<br>
> >><br>
> >> I think it /might/ work here, but I don't know/recall the specifics -<br>
> >> perhaps it doesn't fit here for some reason.<br>
> ><br>
> > Ah, I see what you mean. Interestingly, WeakVH already has a<br>
> > specialization of simplify_type, so you would expect this to work:<br>
> ><br>
> >    if (Instruction *Inst =<br>
> >            dyn_cast_or_null<Instruction>(DeadInsts.pop_back_val()))<br>
> ><br>
> > but it doesn't - the rvalue gets type `const llvm::Instruction *` so we<br>
> > get an error for trying to remove the const without a cast. On the other<br>
> > hand, this does work:<br>
> ><br>
> >    WeakVH WVH = DeadInsts.pop_back_val();<br>
> >    if (Instruction *Inst = dyn_cast_or_null<Instruction>(WVH))<br>
> ><br>
> > So for whatever reason the template specialization chooses the const<br>
> > version (even though the non-const version is also viable) when we give<br>
> > it an rvalue. I guess this is a bug or limitation in simplify_type.<br>
><br>
> Just needed another template specialization for `const WeakVH`.  I added<br>
> one (and used it in IndVarSimplify) in r240599.<br>
><br>
> Is this a bug in the way the traits work - such that it should work for const and non-const? Something we could fix more generally? (or should we make it so that a specialization for simplify_type of const T also works for T? So shallow-const handles like this only need one specialization)<br>
<br>
</div></div>I wouldn't say it's a bug, but I like the idea of making the `const T`<br>
version work for `T` if there isn't one available.<br>
<br>
Would this just work?<br>
<br>
    template <class T> struct simplify_type<T></blockquote><div><br>That would be an unspecialized specialization.<br><br>So we could do it the other way:<br><br>template<typename T> struct simplify_type<const T> : simplify_type<T> {}; <br><br>But that won't work for all types (& you're right, maybe that's just a matter of diagnostic quality, not functional correctness)...<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> : simplify_type<const T> {};<br>
<br>
My guess is that we'd really want an `enable_if<>` in there somewhere<br>
to keep the compiler errors sane though.  Not exactly sure.<br>
<div class="HOEnZb"><div class="h5"><br>
><br>
><br>
> > As it stands, I find the static_cast clearer than the implicit<br>
> > conversion, especially since the obvious transformation of getting rid<br>
> > of the temp variable doesn't work with the implicit conversion, so I'm<br>
> > going to leave this as is.<br>
> > _______________________________________________<br>
> > llvm-commits mailing list<br>
> > <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> > <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br>
</div></div></blockquote></div><br></div></div>