Commit 0c704176 by Manasij Mukherjee

Selectively invert ICMP operands for better address optimization

Results in lower code size and more loads folded into cmp instructions. BUG=none R=eholk@chromium.org, jpp@chromium.org, stichnot@chromium.org Review URL: https://codereview.chromium.org/2124973005 .
parent 752e59fa
...@@ -65,9 +65,10 @@ const struct InstFcmpAttributes_ { ...@@ -65,9 +65,10 @@ const struct InstFcmpAttributes_ {
// Using non-anonymous struct so that array_lengthof works. // Using non-anonymous struct so that array_lengthof works.
const struct InstIcmpAttributes_ { const struct InstIcmpAttributes_ {
const char *DisplayString; const char *DisplayString;
InstIcmp::ICond Reverse;
} InstIcmpAttributes[] = { } InstIcmpAttributes[] = {
#define X(tag, str) \ #define X(tag, reverse, str) \
{ str } \ { str, InstIcmp::ICond::reverse } \
, ,
ICEINSTICMP_TABLE ICEINSTICMP_TABLE
#undef X #undef X
...@@ -1085,6 +1086,10 @@ void InstTarget::dump(const Cfg *Func) const { ...@@ -1085,6 +1086,10 @@ void InstTarget::dump(const Cfg *Func) const {
InstBreakpoint::InstBreakpoint(Cfg *Func) InstBreakpoint::InstBreakpoint(Cfg *Func)
: InstHighLevel(Func, Inst::Breakpoint, 0, nullptr) {} : InstHighLevel(Func, Inst::Breakpoint, 0, nullptr) {}
void InstIcmp::reverseConditionAndOperands() {
Condition = InstIcmpAttributes[Condition].Reverse;
std::swap(Srcs[0], Srcs[1]);
}
bool checkForRedundantAssign(const Variable *Dest, const Operand *Source) { bool checkForRedundantAssign(const Variable *Dest, const Operand *Source) {
const auto *SrcVar = llvm::dyn_cast<const Variable>(Source); const auto *SrcVar = llvm::dyn_cast<const Variable>(Source);
if (!SrcVar) if (!SrcVar)
......
...@@ -31,74 +31,74 @@ ...@@ -31,74 +31,74 @@
// representable in the destination format. This standard does not // representable in the destination format. This standard does not
// specify which of the input NaNs will provide the payload." // specify which of the input NaNs will provide the payload."
#define ICEINSTARITHMETIC_TABLE \ #define ICEINSTARITHMETIC_TABLE \
/* enum value, printable string, commutative */ \ /* enum value, printable string, commutative */ \
X(Add, "add", 1) \ X(Add, "add", 1) \
X(Fadd, "fadd", 1) \ X(Fadd, "fadd", 1) \
X(Sub, "sub", 0) \ X(Sub, "sub", 0) \
X(Fsub, "fsub", 0) \ X(Fsub, "fsub", 0) \
X(Mul, "mul", 1) \ X(Mul, "mul", 1) \
X(Fmul, "fmul", 1) \ X(Fmul, "fmul", 1) \
X(Udiv, "udiv", 0) \ X(Udiv, "udiv", 0) \
X(Sdiv, "sdiv", 0) \ X(Sdiv, "sdiv", 0) \
X(Fdiv, "fdiv", 0) \ X(Fdiv, "fdiv", 0) \
X(Urem, "urem", 0) \ X(Urem, "urem", 0) \
X(Srem, "srem", 0) \ X(Srem, "srem", 0) \
X(Frem, "frem", 0) \ X(Frem, "frem", 0) \
X(Shl, "shl", 0) \ X(Shl, "shl", 0) \
X(Lshr, "lshr", 0) \ X(Lshr, "lshr", 0) \
X(Ashr, "ashr", 0) \ X(Ashr, "ashr", 0) \
X(And, "and", 1) \ X(And, "and", 1) \
X(Or, "or", 1) \ X(Or, "or", 1) \
X(Xor, "xor", 1) X(Xor, "xor", 1)
//#define X(tag, str, commutative) //#define X(tag, str, commutative)
#define ICEINSTCAST_TABLE \ #define ICEINSTCAST_TABLE \
/* enum value, printable string */ \ /* enum value, printable string */ \
X(Trunc, "trunc") \ X(Trunc, "trunc") \
X(Zext, "zext") \ X(Zext, "zext") \
X(Sext, "sext") \ X(Sext, "sext") \
X(Fptrunc, "fptrunc") \ X(Fptrunc, "fptrunc") \
X(Fpext, "fpext") \ X(Fpext, "fpext") \
X(Fptoui, "fptoui") \ X(Fptoui, "fptoui") \
X(Fptosi, "fptosi") \ X(Fptosi, "fptosi") \
X(Uitofp, "uitofp") \ X(Uitofp, "uitofp") \
X(Sitofp, "sitofp") \ X(Sitofp, "sitofp") \
X(Bitcast, "bitcast") X(Bitcast, "bitcast")
//#define X(tag, str) //#define X(tag, str)
#define ICEINSTFCMP_TABLE \ #define ICEINSTFCMP_TABLE \
/* enum value, printable string */ \ /* enum value, printable string */ \
X(False, "false") \ X(False, "false") \
X(Oeq, "oeq") \ X(Oeq, "oeq") \
X(Ogt, "ogt") \ X(Ogt, "ogt") \
X(Oge, "oge") \ X(Oge, "oge") \
X(Olt, "olt") \ X(Olt, "olt") \
X(Ole, "ole") \ X(Ole, "ole") \
X(One, "one") \ X(One, "one") \
X(Ord, "ord") \ X(Ord, "ord") \
X(Ueq, "ueq") \ X(Ueq, "ueq") \
X(Ugt, "ugt") \ X(Ugt, "ugt") \
X(Uge, "uge") \ X(Uge, "uge") \
X(Ult, "ult") \ X(Ult, "ult") \
X(Ule, "ule") \ X(Ule, "ule") \
X(Une, "une") \ X(Une, "une") \
X(Uno, "uno") \ X(Uno, "uno") \
X(True, "true") X(True, "true")
//#define X(tag, str) //#define X(tag, str)
#define ICEINSTICMP_TABLE \ #define ICEINSTICMP_TABLE \
/* enum value, printable string */ \ /* enum value, reverse, printable string */ \
X(Eq, "eq") \ X(Eq, Eq, "eq") \
X(Ne, "ne") \ X(Ne, Ne, "ne") \
X(Ugt, "ugt") \ X(Ugt, Ult, "ugt") \
X(Uge, "uge") \ X(Uge, Ule, "uge") \
X(Ult, "ult") \ X(Ult, Ugt, "ult") \
X(Ule, "ule") \ X(Ule, Uge, "ule") \
X(Sgt, "sgt") \ X(Sgt, Slt, "sgt") \
X(Sge, "sge") \ X(Sge, Sle, "sge") \
X(Slt, "slt") \ X(Slt, Sgt, "slt") \
X(Sle, "sle") X(Sle, Sge, "sle")
//#define X(tag, str) //#define X(tag, reverse, str)
#endif // SUBZERO_SRC_ICEINST_DEF #endif // SUBZERO_SRC_ICEINST_DEF
...@@ -538,7 +538,7 @@ class InstIcmp : public InstHighLevel { ...@@ -538,7 +538,7 @@ class InstIcmp : public InstHighLevel {
public: public:
enum ICond { enum ICond {
#define X(tag, str) tag, #define X(tag, inverse, str) tag,
ICEINSTICMP_TABLE ICEINSTICMP_TABLE
#undef X #undef X
_num _num
...@@ -550,6 +550,7 @@ public: ...@@ -550,6 +550,7 @@ public:
InstIcmp(Func, Condition, Dest, Source1, Source2); InstIcmp(Func, Condition, Dest, Source1, Source2);
} }
ICond getCondition() const { return Condition; } ICond getCondition() const { return Condition; }
void reverseConditionAndOperands();
bool isMemoryWrite() const override { return false; } bool isMemoryWrite() const override { return false; }
void dump(const Cfg *Func) const override; void dump(const Cfg *Func) const override;
static bool classof(const Inst *Instr) { return Instr->getKind() == Icmp; } static bool classof(const Inst *Instr) { return Instr->getKind() == Icmp; }
...@@ -558,7 +559,7 @@ private: ...@@ -558,7 +559,7 @@ private:
InstIcmp(Cfg *Func, ICond Condition, Variable *Dest, Operand *Source1, InstIcmp(Cfg *Func, ICond Condition, Variable *Dest, Operand *Source1,
Operand *Source2); Operand *Source2);
const ICond Condition; ICond Condition;
}; };
/// InsertElement instruction. /// InsertElement instruction.
......
...@@ -344,6 +344,7 @@ void TargetLowering::genTargetHelperCalls() { ...@@ -344,6 +344,7 @@ void TargetLowering::genTargetHelperCalls() {
} }
void TargetLowering::doAddressOpt() { void TargetLowering::doAddressOpt() {
doAddressOptOther();
if (llvm::isa<InstLoad>(*Context.getCur())) if (llvm::isa<InstLoad>(*Context.getCur()))
doAddressOptLoad(); doAddressOptLoad();
else if (llvm::isa<InstStore>(*Context.getCur())) else if (llvm::isa<InstStore>(*Context.getCur()))
......
...@@ -396,6 +396,8 @@ protected: ...@@ -396,6 +396,8 @@ protected:
virtual void genTargetHelperCallFor(Inst *Instr) = 0; virtual void genTargetHelperCallFor(Inst *Instr) = 0;
virtual uint32_t getCallStackArgumentsSizeBytes(const InstCall *Instr) = 0; virtual uint32_t getCallStackArgumentsSizeBytes(const InstCall *Instr) = 0;
/// Opportunity to modify other instructions to help Address Optimization
virtual void doAddressOptOther() {}
virtual void doAddressOptLoad() {} virtual void doAddressOptLoad() {}
virtual void doAddressOptStore() {} virtual void doAddressOptStore() {}
virtual void doMockBoundsCheck(Operand *) {} virtual void doMockBoundsCheck(Operand *) {}
......
...@@ -170,7 +170,8 @@ enum _icmp_ll_enum { ...@@ -170,7 +170,8 @@ enum _icmp_ll_enum {
_num _num
}; };
// Define a set of constants based on high-level table entries. // Define a set of constants based on high-level table entries.
#define X(tag, str) static constexpr int _icmp_hl_##tag = InstIcmp::tag; #define X(tag, reverse, str) \
static constexpr int _icmp_hl_##tag = InstIcmp::tag;
ICEINSTICMP_TABLE ICEINSTICMP_TABLE
#undef X #undef X
// Define a set of constants based on low-level table entries, and ensure the // Define a set of constants based on low-level table entries, and ensure the
...@@ -183,7 +184,7 @@ ICMPARM32_TABLE ...@@ -183,7 +184,7 @@ ICMPARM32_TABLE
#undef X #undef X
// Repeat the static asserts with respect to the high-level table entries in // Repeat the static asserts with respect to the high-level table entries in
// case the high-level table has extra entries. // case the high-level table has extra entries.
#define X(tag, str) \ #define X(tag, reverse, str) \
static_assert( \ static_assert( \
_icmp_hl_##tag == _icmp_ll_##tag, \ _icmp_hl_##tag == _icmp_ll_##tag, \
"Inconsistency between ICMPARM32_TABLE and ICEINSTICMP_TABLE: " #tag); "Inconsistency between ICMPARM32_TABLE and ICEINSTICMP_TABLE: " #tag);
......
...@@ -436,7 +436,7 @@ enum _tmp_enum { ...@@ -436,7 +436,7 @@ enum _tmp_enum {
_num _num
}; };
// Define a set of constants based on high-level table entries. // Define a set of constants based on high-level table entries.
#define X(tag, str) static const int _table1_##tag = InstIcmp::tag; #define X(tag, reverse, str) static const int _table1_##tag = InstIcmp::tag;
ICEINSTICMP_TABLE ICEINSTICMP_TABLE
#undef X #undef X
// Define a set of constants based on low-level table entries, and ensure the // Define a set of constants based on low-level table entries, and ensure the
...@@ -450,7 +450,7 @@ ICMPX8632_TABLE ...@@ -450,7 +450,7 @@ ICMPX8632_TABLE
#undef X #undef X
// Repeat the static asserts with respect to the high-level table entries in // Repeat the static asserts with respect to the high-level table entries in
// case the high-level table has extra entries. // case the high-level table has extra entries.
#define X(tag, str) \ #define X(tag, reverse, str) \
static_assert( \ static_assert( \
_table1_##tag == _table2_##tag, \ _table1_##tag == _table2_##tag, \
"Inconsistency between ICMPX8632_TABLE and ICEINSTICMP_TABLE"); "Inconsistency between ICMPX8632_TABLE and ICEINSTICMP_TABLE");
......
...@@ -775,7 +775,7 @@ enum _tmp_enum { ...@@ -775,7 +775,7 @@ enum _tmp_enum {
_num _num
}; };
// Define a set of constants based on high-level table entries. // Define a set of constants based on high-level table entries.
#define X(tag, str) static const int _table1_##tag = InstIcmp::tag; #define X(tag, reverse, str) static const int _table1_##tag = InstIcmp::tag;
ICEINSTICMP_TABLE ICEINSTICMP_TABLE
#undef X #undef X
// Define a set of constants based on low-level table entries, and ensure the // Define a set of constants based on low-level table entries, and ensure the
...@@ -789,7 +789,7 @@ ICMPX8664_TABLE ...@@ -789,7 +789,7 @@ ICMPX8664_TABLE
#undef X #undef X
// Repeat the static asserts with respect to the high-level table entries in // Repeat the static asserts with respect to the high-level table entries in
// case the high-level table has extra entries. // case the high-level table has extra entries.
#define X(tag, str) \ #define X(tag, reverse, str) \
static_assert( \ static_assert( \
_table1_##tag == _table2_##tag, \ _table1_##tag == _table2_##tag, \
"Inconsistency between ICMPX8664_TABLE and ICEINSTICMP_TABLE"); "Inconsistency between ICMPX8664_TABLE and ICEINSTICMP_TABLE");
......
...@@ -289,6 +289,7 @@ protected: ...@@ -289,6 +289,7 @@ protected:
// <Relocatable + Offset>(Base, Index, Shift) // <Relocatable + Offset>(Base, Index, Shift)
X86OperandMem *computeAddressOpt(const Inst *Instr, Type MemType, X86OperandMem *computeAddressOpt(const Inst *Instr, Type MemType,
Operand *Addr); Operand *Addr);
void doAddressOptOther() override;
void doAddressOptLoad() override; void doAddressOptLoad() override;
void doAddressOptStore() override; void doAddressOptStore() override;
void doMockBoundsCheck(Operand *Opnd) override; void doMockBoundsCheck(Operand *Opnd) override;
......
...@@ -5658,6 +5658,39 @@ void TargetX86Base<TraitsType>::lowerLoad(const InstLoad *Load) { ...@@ -5658,6 +5658,39 @@ void TargetX86Base<TraitsType>::lowerLoad(const InstLoad *Load) {
} }
template <typename TraitsType> template <typename TraitsType>
void TargetX86Base<TraitsType>::doAddressOptOther() {
// Inverts some Icmp instructions which helps doAddressOptLoad later.
// TODO(manasijm): Refactor to unify the conditions for Var0 and Var1
Inst *Instr = Context.getCur();
auto *VMetadata = Func->getVMetadata();
if (auto *Icmp = llvm::dyn_cast<InstIcmp>(Instr)) {
if (llvm::isa<Constant>(Icmp->getSrc(0)) ||
llvm::isa<Constant>(Icmp->getSrc(1)))
return;
auto *Var0 = llvm::dyn_cast<Variable>(Icmp->getSrc(0));
if (Var0 == nullptr)
return;
if (!VMetadata->isTracked(Var0))
return;
auto *Op0Def = VMetadata->getFirstDefinitionSingleBlock(Var0);
if (Op0Def == nullptr || !llvm::isa<InstLoad>(Op0Def))
return;
if (VMetadata->getLocalUseNode(Var0) != Context.getNode())
return;
auto *Var1 = llvm::dyn_cast<Variable>(Icmp->getSrc(1));
if (Var1 != nullptr && VMetadata->isTracked(Var1)) {
auto *Op1Def = VMetadata->getFirstDefinitionSingleBlock(Var1);
if (Op1Def != nullptr && !VMetadata->isMultiBlock(Var1) &&
llvm::isa<InstLoad>(Op1Def)) {
return; // Both are loads
}
}
Icmp->reverseConditionAndOperands();
}
}
template <typename TraitsType>
void TargetX86Base<TraitsType>::doAddressOptLoad() { void TargetX86Base<TraitsType>::doAddressOptLoad() {
Inst *Instr = Context.getCur(); Inst *Instr = Context.getCur();
Operand *Addr = Instr->getSrc(0); Operand *Addr = Instr->getSrc(0);
......
...@@ -187,3 +187,20 @@ entry: ...@@ -187,3 +187,20 @@ entry:
; CHECK: or ; CHECK: or
; CHECK-NOT: movss xmm{{[0-9]+}},DWORD PTR [{{e..}}*4+0xFFFF] ; CHECK-NOT: movss xmm{{[0-9]+}},DWORD PTR [{{e..}}*4+0xFFFF]
} }
define internal void @invert_icmp(i32* %arg1, i32* %arg2) {
entry:
%addr.other = load i32, i32* %arg2, align 1
br label %next
next:
%addr.load = load i32, i32* %arg1, align 1
%cond = icmp slt i32 %addr.load, %addr.other
br i1 %cond, label %if.then, label %if.else
if.then:
ret void
if.else:
ret void
; CHECK-LABEL: invert_icmp
; CHECK: cmp {{e..}},DWORD PTR [{{e..}}]
; CHECK: jle
}
\ No newline at end of file
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment