Skip to content

[TableGen][SystemZ] Correctly check the range of a leaf immediate #119931

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions llvm/lib/Target/SystemZ/SystemZInstrInfo.td
Original file line number Diff line number Diff line change
Expand Up @@ -951,10 +951,10 @@ def IILL : BinaryRI<"iill", 0xA53, insertll, GR32, imm32ll16>;
def IILH : BinaryRI<"iilh", 0xA52, insertlh, GR32, imm32lh16>;
def IIHL : BinaryRI<"iihl", 0xA51, insertll, GRH32, imm32ll16>;
def IIHH : BinaryRI<"iihh", 0xA50, insertlh, GRH32, imm32lh16>;
def IILL64 : BinaryAliasRI<insertll, GR64, imm64ll16>;
def IILH64 : BinaryAliasRI<insertlh, GR64, imm64lh16>;
def IIHL64 : BinaryAliasRI<inserthl, GR64, imm64hl16>;
def IIHH64 : BinaryAliasRI<inserthh, GR64, imm64hh16>;
def IILL64 : BinaryAliasRI<insertll64, GR64, imm64ll16>;
def IILH64 : BinaryAliasRI<insertlh64, GR64, imm64lh16>;
def IIHL64 : BinaryAliasRI<inserthl64, GR64, imm64hl16>;
def IIHH64 : BinaryAliasRI<inserthh64, GR64, imm64hh16>;

// ...likewise for 32-bit immediates. For GR32s this is a general
// full-width move. (We use IILF rather than something like LLILF
Expand Down
19 changes: 11 additions & 8 deletions llvm/lib/Target/SystemZ/SystemZOperators.td
Original file line number Diff line number Diff line change
Expand Up @@ -759,14 +759,17 @@ defm block_xor : block_op<xor>;
// Insertions.
def inserti8 : PatFrag<(ops node:$src1, node:$src2),
(or (and node:$src1, -256), node:$src2)>;
def insertll : PatFrag<(ops node:$src1, node:$src2),
(or (and node:$src1, 0xffffffffffff0000), node:$src2)>;
def insertlh : PatFrag<(ops node:$src1, node:$src2),
(or (and node:$src1, 0xffffffff0000ffff), node:$src2)>;
def inserthl : PatFrag<(ops node:$src1, node:$src2),
(or (and node:$src1, 0xffff0000ffffffff), node:$src2)>;
def inserthh : PatFrag<(ops node:$src1, node:$src2),
(or (and node:$src1, 0x0000ffffffffffff), node:$src2)>;

class inserti16<int mask> : PatFrag<(ops node:$src1, node:$src2),
(or (and node:$src1, mask), node:$src2)>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me this pattern isn't really restricted to 16 bits, but could be used to implement all the insertion fragments (8-, 16- and 32-bit). Might be an additional simplification. Either way, LGTM. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in #119962


def insertll : inserti16<0xffff0000>;
def insertlh : inserti16<0x0000ffff>;
def insertll64 : inserti16<0xffffffffffff0000>;
def insertlh64 : inserti16<0xffffffff0000ffff>;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one doesn't fit in i32 and triggered the error. Other changes are for consistency.

def inserthl64 : inserti16<0xffff0000ffffffff>;
def inserthh64 : inserti16<0x0000ffffffffffff>;

def insertlf : PatFrag<(ops node:$src1, node:$src2),
(or (and node:$src1, 0xffffffff00000000), node:$src2)>;
def inserthf : PatFrag<(ops node:$src1, node:$src2),
Expand Down
20 changes: 8 additions & 12 deletions llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2463,20 +2463,16 @@ bool TreePatternNode::ApplyTypeConstraints(TreePattern &TP, bool NotRegisters) {
// Can only check for types of a known size
if (VT == MVT::iPTR)
continue;
unsigned Size = MVT(VT).getFixedSizeInBits();
// Make sure that the value is representable for this type.
if (Size >= 32)
continue;

// Check that the value doesn't use more bits than we have. It must
// either be a sign- or zero-extended equivalent of the original.
int64_t SignBitAndAbove = II->getValue() >> (Size - 1);
if (SignBitAndAbove == -1 || SignBitAndAbove == 0 ||
SignBitAndAbove == 1)
continue;

TP.error("Integer value '" + Twine(II->getValue()) +
"' is out of range for type '" + getEnumName(VT) + "'!");
break;
unsigned Width = MVT(VT).getFixedSizeInBits();
int64_t Val = II->getValue();
if (!isIntN(Width, Val) && !isUIntN(Width, Val)) {
TP.error("Integer value '" + Twine(Val) +
"' is out of range for type '" + getEnumName(VT) + "'!");
break;
}
}
return MadeChange;
}
Expand Down
Loading