[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