[PATCH] D17942: [AArch64] Optimize test and branch sequence when the test's constant operand is power of 2
Chad Rosier via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 8 06:10:33 PST 2016
mcrosier added a comment.
Please run clang-format on you patch, Balaram.
================
Comment at: lib/Target/AArch64/AArch64InstrInfo.cpp:2956
@@ -2955,1 +2955,3 @@
///
+/// \brief Replace test and branch sequence by TBZ/TBNZ instruction when
+/// the test's constant operand is power of 2.
----------------
s/test/compare/
I. e., Replace compare and branch sequence by..?
================
Comment at: lib/Target/AArch64/AArch64InstrInfo.cpp:3015
@@ -3005,1 +3014,3 @@
+ // Look for AND if MI is CBZ/CBNZ and DefMI is a COPY instruction
+ if (!IsTestAndBranch && DefMI->isCopy()) {
----------------
Please add a period. It might also be helpful to comment your converting this sequence to tbz/tbnz when the AND is a constant power of 2.
================
Comment at: lib/Target/AArch64/AArch64InstrInfo.cpp:3029
@@ +3028,3 @@
+ if (!AndDefMI) return false;
+ if (!(AndDefMI->getOpcode() == AArch64::ANDWri) &&
+ !(AndDefMI->getOpcode() == AArch64::ANDXri))
----------------
Might be easier to read as: Opc != ANDW && Opc != ANDX. This should also remove the unneeded parens.
================
Comment at: lib/Target/AArch64/AArch64InstrInfo.cpp:3052
@@ +3051,3 @@
+
+ if (Imm > 31) {
+ MRI->constrainRegClass(NewReg, &AArch64::GPR64RegClass);
----------------
Should you be checking the Imm value here? Isn't it possible to have an Imm <= 31, but still be defining a 64bit register?
I'm wondering if the condition should be something more like (AndDefMI->getOpcode() == AArch64::ANDXri), as is done above?
================
Comment at: lib/Target/AArch64/AArch64InstrInfo.cpp:3054
@@ +3053,3 @@
+ MRI->constrainRegClass(NewReg, &AArch64::GPR64RegClass);
+ if (IsNegativeBranch) {
+ Opc = AArch64::TBNZX;
----------------
Single statement; No need for curly brackets.
You could also use a select statement.
================
Comment at: lib/Target/AArch64/AArch64InstrInfo.cpp:3061
@@ +3060,3 @@
+ MRI->constrainRegClass(NewReg, &AArch64::GPR32RegClass);
+ if (IsNegativeBranch) {
+ Opc = AArch64::TBNZW;
----------------
Same.
http://reviews.llvm.org/D17942
More information about the llvm-commits
mailing list