[PATCH] D13455: [safestack] Fast access to the unsafe stack pointer on AArch64/Android.

[Evgeniy Stepanov via llvm-commits]

eugenis added inline comments.

================
Comment at: lib/CodeGen/TargetLoweringBase.cpp:1665-1666
@@ -1664,1 +1664,4 @@
 
+/// Android provides a libc function to retrieve the address of the current
+/// thread's unsafe stack pointer.
+Value *TargetLoweringBase::getSafeStackPointerLocation(IRBuilder<> &IRB) const {
----------------
echristo wrote:
> eugenis wrote:
> > rengolin wrote:
> > > echristo wrote:
> > > > This comment should go with the Android bits below and a generic comment above the function. Might be better to just duplicate the code in every target rather than have a generic one only implemented for for android (i.e. if there were a generic function in compiler-rt it might make more sense).
> > > Especially when no one it using it...
> > Well there is a generic function in compiler-rt. Or, rather, a thread-local variable. See the remaining part of SafeStack::getOrCreateUnsafeStackPtr which could be moved here as well, and then this function would apply to all imaginable targets.
> > 
> SGTM. Let's see what that looks like.
> 
> It does give the assumption that anyone using safe stack is using compiler-rt, but...
If it's not a known platform (android) and not using compiler-rt, there is nothing we can do anyway.

================
Comment at: lib/Target/AArch64/AArch64ISelLowering.cpp:9761
@@ +9760,3 @@
+
+  const unsigned kTlsOffset = 0x48;
+  Module *M = IRB.GetInsertBlock()->getParent()->getParent();
----------------
echristo wrote:
> Non-standard naming.
Should it be just "TlsOffset", without the "k" prefix?
CodingStandards does not say anything special about constants.


Repository:
  rL LLVM

http://reviews.llvm.org/D13455