Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h | ||
---|---|---|
1046–1070 ↗ | (On Diff #547226) | Is it possible to separate this from the lowerEXT parts? |
llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp | ||
5981 | This shouldn't need to consider the legality of intermediate ops and fail, just unconditionally do this and let the new artifacts legalize |
Split combiner into a seperate commit.
Unconditionally lowers Extends, independent of the intermediate instruction
llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp | ||
---|---|---|
5982 | Can reduce the indenting now. | |
llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp | ||
521 | Can you add a comment here explaining that this is trying to convert larger extends into two smaller ones. | |
523 | (Query.Types[0].isVector()) -> Query.Types[0].isVector() | |
llvm/test/CodeGen/AArch64/GlobalISel/legalize-build-vector.mir | ||
144–145 | Is this with update_mir_test_checks? Maybe just replace %ext with %3 in the code below. |
A few comments but otherwise LGTM, thanks for the new tests.
llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp | ||
---|---|---|
5981–5982 | No need for MidRegister, this can be simplified to: auto NewExt = MIRBuilder.buildInstr(MI.getOpcode(), {MidTy}, {Src}); ... auto UnmergeSrc = MIRBuilder.buildUnmerge(EltTy, NewExt); ...` | |
5988–5996 | Same here, it's less verbose to just create the LLTs and pass those to buildInstr() instead of passing Registers. // ZExt the vectors Register ZExtResTy = DstTy.changeElementCount(DstTy.getElementCount().divideCoefficientBy(2)); auto ZExtRes1 = MIRBuilder.buildInstr(MI.getOpcode(), {ZExtResTy}, {UnmergeSrc.getReg(0)}); auto ZExtRes2 = MIRBuilder.buildInstr(MI.getOpcode(), {ZExtResTy}, {UnmergeSrc.getReg(1)}); |
Rectified comments made by aemerson
Instructions are built using register types instead of creating a register for the instruction to be built
This shouldn't need to consider the legality of intermediate ops and fail, just unconditionally do this and let the new artifacts legalize