[PATCH] D92089: [PowerPC] Materialize i64 constants by enumerated patterns.
Qing Shan Zhang via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 1 21:34:40 PST 2020
steven.zhang added a comment.
I think, this patch overrall looks great as it simplify our logic of the instruction selection for i64 imm. Please update the check with isS[U]Int<>() to make the code more clear. And I assume that, you have run the bmk and bootstrap with this patch.
================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:803
+bool findContiguousZeros(uint64_t Imm, unsigned Num, unsigned &Shift) {
+ unsigned Hi_tz = countTrailingZeros<uint32_t>(Hi_32(Imm));
----------------
It should be static at least. And please add necessary documentation on the semantics for this function.
================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:813
+
+static SDNode *selectI64ImmByPattern(SelectionDAG *CurDAG, const SDLoc &dl,
+ uint64_t Imm, unsigned *InstCnt) {
----------------
I prefer to use reference for the InstrCnt and if for the case that we don't need it, just pass a dummy cnt.
================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:835
+ // {ones}{15-bit valve}
+ if ((64 - LZ) < 16 || (64 - LO) < 16) {
+ SDValue SDImm = CurDAG->getTargetConstant(Imm, dl, MVT::i64);
----------------
Is it more clear to use isInt<N>() ?
================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:841
+ // {ones}{15-bit valve}{16 zeros}
+ if (TZ > 15 && (LZ > 32 || LO > 32))
+ return CurDAG->getMachineNode(PPC::LIS8, dl, MVT::i64,
----------------
Is it more clear to use: TZ > 15 && isInt<16>(Imm >> 16) ?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92089/new/
https://reviews.llvm.org/D92089
More information about the llvm-commits
mailing list