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

[Phoebe Wang via Phabricator via llvm-commits]

pengfei added inline comments.


================
Comment at: llvm/include/llvm/LinkAllPasses.h:180
       (void) llvm::createTailCallEliminationPass();
+      (void)llvm::createTLSVariableHoistPass();
       (void) llvm::createJumpThreadingPass();
----------------
Keep the same format looks better. Up to you.


================
Comment at: llvm/include/llvm/Transforms/Scalar.h:431
+//
+// TLSVariableHoist - This pass prepares a function for expensive TLSVariable.
+//
----------------
What's "prepares a function" mean?


================
Comment at: llvm/include/llvm/Transforms/Scalar/TLSVariableHoist.h:1
+//==- TLSVariableHoist.h ------ Remove Redundant TLS Loads -----*- C++ -*-==//
+//
----------------
Still less than 80.


================
Comment at: llvm/include/llvm/Transforms/Scalar/TLSVariableHoist.h:21
+// will generate Redundant TLS Loads by compiling it with
+// Clang++ -fPIC -ftls-model=global-dynamic -O2 -S
+//
----------------
clang


================
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
----------------
Maybe preserve loop infor too. The pass does nothing with it. This may help with the following passes not run it again.


================
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}
----------------
There meta data is annoying, especially the OneAPI info here doesn't make sense to llvm. The same below.


================
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
----------------
Add `nounwind` in attributes to avoid cfi directives.


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

https://reviews.llvm.org/D120000