This is an archive of the discontinued LLVM Phabricator instance.

[SelectionDAG] Correctly widen bitcast of scalar to vector for big endian
ClosedPublic

Authored by nemanjai on Dec 19 2022, 7:26 PM.

Details

Summary

For big endian targets that need a node such as this:
v2i8 = bitcast i16:tN

legalized by:

  1. Promoting the i16 input type
  2. Widening the v2i32 result type

The result will be incorrect because the legalizer will promote the input type and then produce a scalar_to_vector from that wider type to a vector of N elements of that type. That puts the desired bits into the low order bytes of element zero and they need to be in the high order bytes on big endian systems.

Diff Detail

Event Timeline

nemanjai created this revision.Dec 19 2022, 7:26 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 19 2022, 7:26 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
nemanjai requested review of this revision.Dec 19 2022, 7:26 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 19 2022, 7:26 PM

Note: this fixes the build bot break triggered by https://reviews.llvm.org/rG96d3c82645cf

efriedma added inline comments.Dec 20 2022, 12:13 PM
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
4872

I'd prefer EVT OrigInVT = N->getOperand(0).getValueType();, or something like that.

Can we avoid explicitly forking on endianness here? We should be able to use the same vector type for little-endian, I think. Keeping the DAG as close as possible should make it easier to generate optimal code for targets that have both little-endian and big-endian variants.

I'm a little concerned we could end up producing a weird vector type here; the result of promotion is always going to be a nice type, but the original type could be something less nice. I guess we end up using the CreateStackStoreLoad codepath if we end up choosing an illegal vector type, so maybe we can ignore that for now.

llvm/test/CodeGen/PowerPC/widen-vec-correctly-be.ll
53

Please avoid writing tests that explicitly branch on poison, to make the output more stable.

nemanjai updated this revision to Diff 486880.Jan 6 2023, 7:40 AM
nemanjai edited the summary of this revision. (Show Details)

The legalization to a vector of N elements of original type is fine for both little and big endian targets. Eliminate the branch based on target endianness.

nemanjai marked 2 inline comments as done.Jan 6 2023, 7:42 AM
This revision is now accepted and ready to land.Jan 20 2023, 10:33 AM