[PATCH] D119010: [AggressiveInstCombine] Recognize table-based ctz implementation and enable it for AARCH64 at -O3
Craig Topper via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 4 09:32:17 PST 2022
craig.topper added inline comments.
================
Comment at: llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp:442
+
+ Type *ElType = LI->getPointerOperandType()->getPointerElementType();
+ if (!ElType->isIntegerTy())
----------------
getPointerElementType() will be deleted when we move to OpaquePtr. Use LI->getType()?
================
Comment at: llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp:450
+
+ Type *GEPPointeeType = GEP->getPointerOperandType()->getPointerElementType();
+ if (!GEPPointeeType->isArrayTy())
----------------
GEP->getSourceElementType() I think.
================
Comment at: llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp:468
+ Value *Idx1 = GEP->idx_begin()->get();
+ Constant *Zero = dyn_cast<Constant>(Idx1);
+ if (!Zero || !Zero->isZeroValue())
----------------
Constant->ConstantInt? And isZeroValue->isZero
================
Comment at: llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp:478
+ uint64_t MulConst, ShiftConst;
+ // FIXME: AArch64 has i64 type for the GEP index, so this match will
+ // probably fail for other targets.
----------------
What does it take to address this FIXME so this will work on other targets?
================
Comment at: llvm/test/Transforms/AggressiveInstCombine/AArch64/lower-table-based-ctz.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -aggressive-instcombine -mtriple aarch64-linux-gnu -S < %s | FileCheck %s
----------------
Why are there less tests than the other patch?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119010/new/
https://reviews.llvm.org/D119010
More information about the llvm-commits
mailing list