This is an archive of the discontinued LLVM Phabricator instance.

Differential D134236

[GlobalISel][Legalizer] Fix lowerSelect() not sign-extending the mask value.
ClosedPublic

Authored by aemerson on Sep 19 2022, 4:25 PM.

Details

Reviewers
arsenm
paquette
Commits
rG78833a43e8d8: [GlobalISel][Legalizer] Fix lowerSelect() not sign-extending the mask value.
Summary

I'm not sure why the SEXT_INREG was gated on a bitwidth check of the mask vs element size.

This fixes a miscompile in chromium's skia library.

Diff Detail

Event Timeline

aemerson created this revision.Sep 19 2022, 4:25 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 19 2022, 4:25 PM
Herald added subscribers: hiraditya, rovka. · View Herald Transcript
aemerson requested review of this revision.Sep 19 2022, 4:25 PM
Herald added a subscriber: wdng. · View Herald TranscriptSep 19 2022, 4:25 PM
Harbormaster completed remote builds in B187632: Diff 461408.Sep 19 2022, 6:38 PM
arsenm accepted this revision.Sep 20 2022, 7:00 AM
This revision is now accepted and ready to land.Sep 20 2022, 7:00 AM
This revision was landed with ongoing or failed builds.Sep 20 2022, 8:40 AM
Closed by commit rG78833a43e8d8: [GlobalISel][Legalizer] Fix lowerSelect() not sign-extending the mask value. (authored by aemerson). · Explain Why
This revision was automatically updated to reflect the committed changes.
aemerson added a commit: rG78833a43e8d8: [GlobalISel][Legalizer] Fix lowerSelect() not sign-extending the mask value..

Revision Contents

PathSize
llvm/
lib/
CodeGen/
GlobalISel/
4 lines
test/
CodeGen/
AArch64/
GlobalISel/
3 lines

Diff 461579

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp

  • This file is larger than 256 KB, so syntax highlighting is disabled by default.
Show First 20 Lines • Show All 7,241 Lines • ▼ Show 20 LinesLegalizerHelper::LegalizeResult LegalizerHelper::lowerSelect(MachineInstr &MI) {
if (MaskTy.isScalar()) { if (MaskTy.isScalar()) {
// Turn the scalar condition into a vector condition mask. // Turn the scalar condition into a vector condition mask.
Register MaskElt = MaskReg; Register MaskElt = MaskReg;
// The condition was potentially zero extended before, but we want a sign // The condition was potentially zero extended before, but we want a sign
// extended boolean. // extended boolean.
if (MaskTy.getSizeInBits() <= DstTy.getScalarSizeInBits() && if (MaskTy != LLT::scalar(1))
MaskTy != LLT::scalar(1)) {
MaskElt = MIRBuilder.buildSExtInReg(MaskTy, MaskElt, 1).getReg(0); MaskElt = MIRBuilder.buildSExtInReg(MaskTy, MaskElt, 1).getReg(0);
}
// Continue the sign extension (or truncate) to match the data type. // Continue the sign extension (or truncate) to match the data type.
MaskElt = MIRBuilder.buildSExtOrTrunc(DstTy.getElementType(), MaskElt = MIRBuilder.buildSExtOrTrunc(DstTy.getElementType(),
MaskElt).getReg(0); MaskElt).getReg(0);
// Generate a vector splat idiom. // Generate a vector splat idiom.
auto ShufSplat = MIRBuilder.buildShuffleSplat(DstTy, MaskElt); auto ShufSplat = MIRBuilder.buildShuffleSplat(DstTy, MaskElt);
MaskReg = ShufSplat.getReg(0); MaskReg = ShufSplat.getReg(0);
▲ Show 20 Lines • Show All 631 Lines • Show Last 20 Lines

llvm/test/CodeGen/AArch64/GlobalISel/legalize-select.mir

Show First 20 Lines • Show All 301 Lines • ▼ Show 20 Lines bb.0:
; CHECK-NEXT: %q2:_(<4 x s32>) = COPY $q2 ; CHECK-NEXT: %q2:_(<4 x s32>) = COPY $q2
; CHECK-NEXT: %vec_cond0:_(<4 x s1>) = G_ICMP intpred(eq), %q0(<4 x s32>), %q1 ; CHECK-NEXT: %vec_cond0:_(<4 x s1>) = G_ICMP intpred(eq), %q0(<4 x s32>), %q1
; CHECK-NEXT: %vec_cond1:_(<4 x s1>) = G_ICMP intpred(eq), %q0(<4 x s32>), %q2 ; CHECK-NEXT: %vec_cond1:_(<4 x s1>) = G_ICMP intpred(eq), %q0(<4 x s32>), %q2
; CHECK-NEXT: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 4100 ; CHECK-NEXT: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 4100
; CHECK-NEXT: [[C1:%[0-9]+]]:_(s32) = G_FCONSTANT float 0.000000e+00 ; CHECK-NEXT: [[C1:%[0-9]+]]:_(s32) = G_FCONSTANT float 0.000000e+00
; CHECK-NEXT: [[BUILD_VECTOR:%[0-9]+]]:_(<4 x s32>) = G_BUILD_VECTOR [[C1]](s32), [[C1]](s32), [[C1]](s32), [[C1]](s32) ; CHECK-NEXT: [[BUILD_VECTOR:%[0-9]+]]:_(<4 x s32>) = G_BUILD_VECTOR [[C1]](s32), [[C1]](s32), [[C1]](s32), [[C1]](s32)
; CHECK-NEXT: %cmp:_(s1) = G_ICMP intpred(eq), %w0(s32), [[C]] ; CHECK-NEXT: %cmp:_(s1) = G_ICMP intpred(eq), %w0(s32), [[C]]
; CHECK-NEXT: [[ZEXT:%[0-9]+]]:_(s32) = G_ZEXT %cmp(s1) ; CHECK-NEXT: [[ZEXT:%[0-9]+]]:_(s32) = G_ZEXT %cmp(s1)
; CHECK-NEXT: [[TRUNC:%[0-9]+]]:_(s1) = G_TRUNC [[ZEXT]](s32) ; CHECK-NEXT: [[SEXT_INREG:%[0-9]+]]:_(s32) = G_SEXT_INREG [[ZEXT]], 1
; CHECK-NEXT: [[TRUNC:%[0-9]+]]:_(s1) = G_TRUNC [[SEXT_INREG]](s32)
; CHECK-NEXT: [[DEF:%[0-9]+]]:_(<4 x s1>) = G_IMPLICIT_DEF ; CHECK-NEXT: [[DEF:%[0-9]+]]:_(<4 x s1>) = G_IMPLICIT_DEF
; CHECK-NEXT: [[C2:%[0-9]+]]:_(s64) = G_CONSTANT i64 0 ; CHECK-NEXT: [[C2:%[0-9]+]]:_(s64) = G_CONSTANT i64 0
; CHECK-NEXT: [[IVEC:%[0-9]+]]:_(<4 x s1>) = G_INSERT_VECTOR_ELT [[DEF]], [[TRUNC]](s1), [[C2]](s64) ; CHECK-NEXT: [[IVEC:%[0-9]+]]:_(<4 x s1>) = G_INSERT_VECTOR_ELT [[DEF]], [[TRUNC]](s1), [[C2]](s64)
; CHECK-NEXT: [[SHUF:%[0-9]+]]:_(<4 x s1>) = G_SHUFFLE_VECTOR [[IVEC]](<4 x s1>), [[DEF]], shufflemask(0, 0, 0, 0) ; CHECK-NEXT: [[SHUF:%[0-9]+]]:_(<4 x s1>) = G_SHUFFLE_VECTOR [[IVEC]](<4 x s1>), [[DEF]], shufflemask(0, 0, 0, 0)
; CHECK-NEXT: [[C3:%[0-9]+]]:_(s1) = G_CONSTANT i1 true ; CHECK-NEXT: [[C3:%[0-9]+]]:_(s1) = G_CONSTANT i1 true
; CHECK-NEXT: [[BUILD_VECTOR1:%[0-9]+]]:_(<4 x s1>) = G_BUILD_VECTOR [[C3]](s1), [[C3]](s1), [[C3]](s1), [[C3]](s1) ; CHECK-NEXT: [[BUILD_VECTOR1:%[0-9]+]]:_(<4 x s1>) = G_BUILD_VECTOR [[C3]](s1), [[C3]](s1), [[C3]](s1), [[C3]](s1)
; CHECK-NEXT: [[XOR:%[0-9]+]]:_(<4 x s1>) = G_XOR [[SHUF]], [[BUILD_VECTOR1]] ; CHECK-NEXT: [[XOR:%[0-9]+]]:_(<4 x s1>) = G_XOR [[SHUF]], [[BUILD_VECTOR1]]
; CHECK-NEXT: [[AND:%[0-9]+]]:_(<4 x s1>) = G_AND %vec_cond0, [[SHUF]] ; CHECK-NEXT: [[AND:%[0-9]+]]:_(<4 x s1>) = G_AND %vec_cond0, [[SHUF]]
▲ Show 20 Lines • Show All 67 Lines • Show Last 20 Lines