This is an archive of the discontinued LLVM Phabricator instance.

Differential D24151

[Legalizer] Don't throw away false low half for GT/LT SETCC
ClosedPublic

Authored by mkuper on Sep 1 2016, 12:32 PM.

Details

Reviewers
spatel
majnemer
Commits
rG7bc54cebeab4: [Legalizer] Don't throw away false low half when expanding GT/LT SETCC
rL280424: [Legalizer] Don't throw away false low half when expanding GT/LT SETCC
Summary

When expanding a SETCC for which the low half is known to evaluate to false, we can only throw it away for LT/GT comparisons, not LE/GE.

This fixes PR29170.

Diff Detail

Event Timeline

mkuper updated this revision to Diff 70050.Sep 1 2016, 12:32 PM
mkuper retitled this revision from to [Legalizer] Don't throw away false low half for GT/LT SETCC.
mkuper updated this object.
mkuper added reviewers: spatel, majnemer.
mkuper added a subscriber: llvm-commits.
spatel accepted this revision.Sep 1 2016, 2:23 PM
spatel edited edge metadata.

LGTM - but there's a bug in the regression test that needs to be fixed.

test/CodeGen/X86/pr29170.ll
10

'CHECL' - use utils/update_test_checks.py to avoid human error? :)

This revision is now accepted and ready to land.Sep 1 2016, 2:23 PM
mkuper added inline comments.Sep 1 2016, 2:27 PM
test/CodeGen/X86/pr29170.ll
10

Yikes, thanks!
(I intentionally didn't use update_test_checks, since I wanted a fairly "loose" test. But should have double checked the checks!)

Closed by commit rL280424: [Legalizer] Don't throw away false low half when expanding GT/LT SETCC (authored by mkuper). · Explain WhySep 1 2016, 4:11 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
lib/
CodeGen/
SelectionDAG/
18 lines
test/
CodeGen/
X86/
32 lines

Diff 70050

lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp

Show First 20 Lines • Show All 2,859 Lines • ▼ Show 20 Lines HiCmp = TLI.SimplifySetCC(getSetCCResultType(LHSHi.getValueType()), LHSHi,
RHSHi, CCCode, false, DagCombineInfo, dl); RHSHi, CCCode, false, DagCombineInfo, dl);
if (!HiCmp.getNode()) if (!HiCmp.getNode())
HiCmp = HiCmp =
DAG.getNode(ISD::SETCC, dl, getSetCCResultType(LHSHi.getValueType()), DAG.getNode(ISD::SETCC, dl, getSetCCResultType(LHSHi.getValueType()),
LHSHi, RHSHi, DAG.getCondCode(CCCode)); LHSHi, RHSHi, DAG.getCondCode(CCCode));
ConstantSDNode *LoCmpC = dyn_cast<ConstantSDNode>(LoCmp.getNode()); ConstantSDNode *LoCmpC = dyn_cast<ConstantSDNode>(LoCmp.getNode());
ConstantSDNode *HiCmpC = dyn_cast<ConstantSDNode>(HiCmp.getNode()); ConstantSDNode *HiCmpC = dyn_cast<ConstantSDNode>(HiCmp.getNode());
if ((LoCmpC && LoCmpC->isNullValue()) ||
(HiCmpC && HiCmpC->isNullValue() && bool EqAllowed = (CCCode == ISD::SETLE || CCCode == ISD::SETGE ||
(CCCode == ISD::SETLE || CCCode == ISD::SETGE || CCCode == ISD::SETUGE || CCCode == ISD::SETUGE || CCCode == ISD::SETULE);
CCCode == ISD::SETULE)) ||
(HiCmpC && HiCmpC->getAPIntValue() == 1 && if ((EqAllowed && (HiCmpC && HiCmpC->isNullValue())) ||
(CCCode == ISD::SETLT || CCCode == ISD::SETGT || CCCode == ISD::SETUGT || (!EqAllowed && ((HiCmpC && (HiCmpC->getAPIntValue() == 1)) ||
CCCode == ISD::SETULT))) { (LoCmpC && LoCmpC->isNullValue())))) {
// low part is known false, returns high part.
// For LE / GE, if high part is known false, ignore the low part. // For LE / GE, if high part is known false, ignore the low part.
// For LT / GT, if high part is known true, ignore the low part. // For LT / GT: if low part is known false, return the high part.
// if high part is known true, ignore the low part.
NewLHS = HiCmp; NewLHS = HiCmp;
NewRHS = SDValue(); NewRHS = SDValue();
return; return;
} }
if (LHSHi == RHSHi) { if (LHSHi == RHSHi) {
// Comparing the low bits is enough. // Comparing the low bits is enough.
NewLHS = LoCmp; NewLHS = LoCmp;
▲ Show 20 Lines • Show All 516 Lines • Show Last 20 Lines

test/CodeGen/X86/pr29170.ll

; RUN: llc < %s | FileCheck %s
target datalayout = "e-m:e-p:32:32-f64:32:64-f80:32-n8:16:32-S128"
target triple = "i386-unknown-linux-gnu"
@b = global i16 0, align 4
; CHECK-LABEL: @main
; CHECK: cmpl
; CHECL: sbbl
spatelUnsubmitted
Not Done
ReplyInline Actions

'CHECL' - use utils/update_test_checks.py to avoid human error? :)

spatel: 'CHECL' - use utils/update_test_checks.py to avoid human error? :)
mkuperAuthorUnsubmitted
Not Done
ReplyInline Actions

Yikes, thanks!
(I intentionally didn't use update_test_checks, since I wanted a fairly "loose" test. But should have double checked the checks!)

mkuper: Yikes, thanks! (I intentionally didn't use update_test_checks, since I wanted a fairly "loose"…
define i32 @main() {
entry:
%true = icmp eq i32 0, 0
%const = bitcast i64 -4294967296 to i64
br i1 %true, label %go, label %if.else
go:
%b = load i16, i16* @b, align 4
%sext = shl i16 %b, 8
%conv = ashr i16 %sext, 8
%neg4 = xor i64 %const, -1
%conv5 = zext i16 %conv to i64
%cmp = icmp slt i64 %conv5, %neg4
br i1 %cmp, label %if.then, label %if.else
if.then:
ret i32 42
if.else:
ret i32 0
}