[PATCH] D35498: [LoopVectorizer] Use two step casting for float to pointer type.

Michael Kuperstein via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 18 10:29:53 PDT 2017


mkuper added a comment.

>> Anyway, I agree this isn't the right fix, but my gut feeling is that the right fix is to actually allow the builder to create a bitcast between a pointer and a pointer-sized float.
>>  I don't see anything in the langref that makes the semantics of this undefined. Renato, what kind of semantic issues do you see here?
> 
> I agree the vectorizer can "safely" assume this case is ok because we know the original datatype was a pointer anyway and this is part of a strided load, but I feel we'd be opening a can of worms if we start allowing any float<->pointer conversion by default.
> 
> For example, a different case would be pointer->float->double->pointer. C has automatic promotions, and some corner cases may slip and create that sequence, which would destroy the bit-pattern and therefore the memory address.
> 
> So, if we can do this in this specific case, Manoj's current fix is "better" than moving it up `CreateBitOrPointerCast`, because we know what the semantics is. Or maybe we just load them as "data" (i32/i64?) and then bitcast safely?
> 
> Makes sense?

First of all, sorry, you're right, I misread the langref - the cast really is illegal in IR, it's not a builder issue.
I really don't think it should be illegal - but a patch trying to fix a crash is probably not the right place to shave that yak,

I guess this is fine as a stop-gap. I think we need the other direction too, though. That is, should cover both the pointer -> float and float -> pointer cases. (And have tests for both.)


https://reviews.llvm.org/D35498





More information about the llvm-commits mailing list