[PATCH] D120000: [1/2] TLS loads opimization (hoist)

[Xiang Zhang via Phabricator via llvm-commits]

xiangzhangllvm marked 7 inline comments as done.
xiangzhangllvm added inline comments.


================
Comment at: llvm/include/llvm/LinkAllPasses.h:180
       (void) llvm::createTailCallEliminationPass();
+      (void)llvm::createTLSVariableHoistPass();
       (void) llvm::createJumpThreadingPass();
----------------
pengfei wrote:
> Keep the same format looks better. Up to you.
Yes, this is follow clang-format, let it be.


================
Comment at: llvm/test/CodeGen/AArch64/O3-pipeline.ll:69
 ; CHECK-NEXT:         Dominator Tree Construction
 ; CHECK-NEXT:         Natural Loop Information
 ; CHECK-NEXT:         Scalar Evolution Analysis
----------------
pengfei wrote:
> Maybe preserve loop infor too. The pass does nothing with it. This may help with the following passes not run it again.
This pass changed the instructions, so BB info should better be updated.


================
Comment at: llvm/test/CodeGen/X86/tls-loads-control.ll:176
+!3 = !{i32 1, !"tls-load-hoist", !"Optimize"}
+!4 = !{!"Intel(R) oneAPI DPC++/C++ Compiler 2022.1.0 (2022.x.0.YYYYMMDD)"}
+!5 = !{!6, !6, i64 0}
----------------
pengfei wrote:
> There meta data is annoying, especially the OneAPI info here doesn't make sense to llvm. The same below.
Done, but I think keep more info is not bad for test, maybe it is personal habits, I usually prefer to sync the *.ll with the *.c/cpp


================
Comment at: llvm/test/CodeGen/X86/tls-loads-control3.ll:160
+; HOIST0-NEXT:    pushq %r15
+; HOIST0-NEXT:    .cfi_def_cfa_offset 16
+; HOIST0-NEXT:    pushq %r14
----------------
pengfei wrote:
> Add `nounwind` in attributes to avoid cfi directives.
Add nounwind to #0, but seems cfi still here, anyway, that is not important, I think.


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

https://reviews.llvm.org/D120000