[PATCH] D113291: [AggressiveInstCombine] Lower Table Based CTTZ and enable it for AARCH64 in -O3

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

> TODO: Get the data on SPEC benchmark.

Did you manage to collect any perf data yet to motivate this change?

The tests already contain a few things that seem unrelated, it would be good to clean those things up.



================
Comment at: llvm/test/Transforms/AggressiveInstCombine/AArch64/dereferencing-pointer.ll:28
+
+; Function Attrs: mustprogress nofree norecurse nosync nounwind readonly uwtable willreturn
+define dso_local i32 @ctz6(i64* nocapture readonly %b) {
----------------
That's out of date. 


================
Comment at: llvm/test/Transforms/AggressiveInstCombine/AArch64/dereferencing-pointer.ll:40
+entry:
+  %0 = load i64, i64* %b, align 8
+  %sub = sub i64 0, %0
----------------
can the tests instead just take `i64 %b` or does it need to be a pointer? (


================
Comment at: llvm/test/Transforms/AggressiveInstCombine/AArch64/dereferencing-pointer.ll:50
+
+!llvm.module.flags = !{!0, !1, !2, !3, !4, !5, !6}
+!llvm.ident = !{!7}
----------------
is all the mote data needed? Same for the other tests


================
Comment at: llvm/test/Transforms/AggressiveInstCombine/AArch64/non-argument-value.ll:31
+; ModuleID = 'x_not_arg.c'
+source_filename = "x_not_arg.c"
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
----------------
not needed


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113291/new/

https://reviews.llvm.org/D113291