[PATCH] D19104: [X86] Extend some Linux special cases to cover kFreeBSD.

Simon Pilgrim via llvm-commits llvm-commits at lists.llvm.org
Mon May 2 06:00:53 PDT 2016


RKSimon added inline comments.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:2004
@@ -2003,3 +2003,3 @@
 Value *X86TargetLowering::getIRStackGuard(IRBuilder<> &IRB) const {
-  if (!Subtarget.isTargetLinux())
+  if (!Subtarget.isTargetGlibc())
     return TargetLowering::getIRStackGuard(IRB);
----------------
Add comments detailing the glibc requirement? A reference if possible similar to the diff anove in X86ISelDAGToDAG.cpp.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:2017
@@ -2016,3 +2016,3 @@
 void X86TargetLowering::insertSSPDeclarations(Module &M) const {
-  if (!Subtarget.isTargetLinux())
+  if (!Subtarget.isTargetGlibc())
     TargetLowering::insertSSPDeclarations(M);
----------------
Add comments detailing the glibc requirement?

================
Comment at: lib/Target/X86/X86Subtarget.cpp:234
@@ -233,3 +233,3 @@
     stackAlignment = StackAlignOverride;
-  else if (isTargetDarwin() || isTargetLinux() || isTargetSolaris() ||
+  else if (isTargetDarwin() || isTargetGlibc() || isTargetSolaris() ||
            In64BitMode)
----------------
Your comment lists the OS types and then you use the GLibc helper - would it be clearer to add a isTargetkFreeBSD() to the if() instead?

An alternative would be to update the comment to make it clear that glibc x86 32-bit targets always have 16-byte alignment, but I'm not sure if that is actually true? If it is, please add a reference.

Additionally, this needs to be tested - I don't think the stack protector tests cover this.


Repository:
  rL LLVM

http://reviews.llvm.org/D19104





More information about the llvm-commits mailing list