[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