Re: [PATCH] D14871: [Power PC] fix calculating address of arguments on stack for variadic functions

2015-12-03 Thread Petar Jovanovic via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL254670: [PowerPC] Fix calculating address of arguments on 
stack for variadic func (authored by petarj).

Changed prior to commit:
  http://reviews.llvm.org/D14871?vs=41613=41826#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D14871

Files:
  cfe/trunk/lib/CodeGen/TargetInfo.cpp
  cfe/trunk/test/CodeGen/ppc-varargs-struct.c
  cfe/trunk/test/CodeGen/x86_64-arguments.c

Index: cfe/trunk/test/CodeGen/ppc-varargs-struct.c
===
--- cfe/trunk/test/CodeGen/ppc-varargs-struct.c
+++ cfe/trunk/test/CodeGen/ppc-varargs-struct.c
@@ -39,9 +39,13 @@
 // CHECK-PPC:[[USING_OVERFLOW]]
 // CHECK-PPC-NEXT:  [[OVERFLOW_AREA_P:%[0-9]+]] = getelementptr inbounds %struct.__va_list_tag, %struct.__va_list_tag* [[ARRAYDECAY]], i32 0, i32 3
 // CHECK-PPC-NEXT:  [[OVERFLOW_AREA:%.+]] = load i8*, i8** [[OVERFLOW_AREA_P]], align 4
-// CHECK-PPC-NEXT:  [[MEMADDR:%.+]] = bitcast i8* [[OVERFLOW_AREA]] to %struct.x**
-// CHECK-PPC-NEXT:  [[NEW_OVERFLOW_AREA:%[0-9]+]] = getelementptr inbounds i8, i8* [[OVERFLOW_AREA]], i32 4
-// CHECK-PPC-NEXT:  store i8* [[NEW_OVERFLOW_AREA]], i8** [[OVERFLOW_AREA_P]]
+// CHECK-PPC-NEXT:  %{{[0-9]+}} =  ptrtoint i8* %argp.cur to i32
+// CHECK-PPC-NEXT:  %{{[0-9]+}} = add i32 %{{[0-9]+}}, 7
+// CHECK-PPC-NEXT:  %{{[0-9]+}} = and i32 %{{[0-9]+}}, -8
+// CHECK-PPC-NEXT:  %argp.cur.aligned = inttoptr i32 %{{[0-9]+}} to i8*
+// CHECK-PPC-NEXT:  [[MEMADDR:%.+]] = bitcast i8* %argp.cur.aligned to %struct.x**
+// CHECK-PPC-NEXT:  [[NEW_OVERFLOW_AREA:%[0-9]+]] = getelementptr inbounds i8, i8* %argp.cur.aligned, i32 4
+// CHECK-PPC-NEXT:  store i8* [[NEW_OVERFLOW_AREA:%[0-9]+]], i8** [[OVERFLOW_AREA_P]], align 4
 // CHECK-PPC-NEXT:  br label %[[CONT]]
 //
 // CHECK-PPC:[[CONT]]
Index: cfe/trunk/test/CodeGen/x86_64-arguments.c
===
--- cfe/trunk/test/CodeGen/x86_64-arguments.c
+++ cfe/trunk/test/CodeGen/x86_64-arguments.c
@@ -336,7 +336,8 @@
 
 // CHECK-LABEL: define i32 @f44
 // CHECK: ptrtoint
-// CHECK-NEXT: and {{.*}}, -32
+// CHECK-NEXT: add i64 %{{[0-9]+}}, 31
+// CHECK-NEXT: and i64 %{{[0-9]+}}, -32
 // CHECK-NEXT: inttoptr
 typedef int T44 __attribute((vector_size(32)));
 struct s44 { T44 x; int y; };
Index: cfe/trunk/lib/CodeGen/TargetInfo.cpp
===
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp
@@ -162,6 +162,23 @@
   OS << ")\n";
 }
 
+// Dynamically round a pointer up to a multiple of the given alignment.
+static llvm::Value *emitRoundPointerUpToAlignment(CodeGenFunction ,
+  llvm::Value *Ptr,
+  CharUnits Align) {
+  llvm::Value *PtrAsInt = Ptr;
+  // OverflowArgArea = (OverflowArgArea + Align - 1) & -Align;
+  PtrAsInt = CGF.Builder.CreatePtrToInt(PtrAsInt, CGF.IntPtrTy);
+  PtrAsInt = CGF.Builder.CreateAdd(PtrAsInt,
+llvm::ConstantInt::get(CGF.IntPtrTy, Align.getQuantity() - 1));
+  PtrAsInt = CGF.Builder.CreateAnd(PtrAsInt,
+   llvm::ConstantInt::get(CGF.IntPtrTy, -Align.getQuantity()));
+  PtrAsInt = CGF.Builder.CreateIntToPtr(PtrAsInt,
+Ptr->getType(),
+Ptr->getName() + ".aligned");
+  return PtrAsInt;
+}
+
 /// Emit va_arg for a platform using the common void* representation,
 /// where arguments are simply emitted in an array of slots on the stack.
 ///
@@ -193,17 +210,10 @@
   // If the CC aligns values higher than the slot size, do so if needed.
   Address Addr = Address::invalid();
   if (AllowHigherAlign && DirectAlign > SlotSize) {
-llvm::Value *PtrAsInt = Ptr;
-PtrAsInt = CGF.Builder.CreatePtrToInt(PtrAsInt, CGF.IntPtrTy);
-PtrAsInt = CGF.Builder.CreateAdd(PtrAsInt,
-  llvm::ConstantInt::get(CGF.IntPtrTy, DirectAlign.getQuantity() - 1));
-PtrAsInt = CGF.Builder.CreateAnd(PtrAsInt,
- llvm::ConstantInt::get(CGF.IntPtrTy, -DirectAlign.getQuantity()));
-Addr = Address(CGF.Builder.CreateIntToPtr(PtrAsInt, Ptr->getType(),
-  "argp.cur.aligned"),
-   DirectAlign);
+Addr = Address(emitRoundPointerUpToAlignment(CGF, Ptr, DirectAlign),
+ DirectAlign);
   } else {
-Addr = Address(Ptr, SlotSize);
+Addr = Address(Ptr, SlotSize); 
   }
 
   // Advance the pointer past the argument, then store that back.
@@ -3072,19 +3082,10 @@
   // byte boundary if alignment needed by type exceeds 8 byte boundary.
   // It isn't stated explicitly in the standard, but in practice we use
   // alignment greater than 16 where necessary.
-  uint64_t Align = CGF.getContext().getTypeAlignInChars(Ty).getQuantity();
-  if (Align > 8) {
-// overflow_arg_area =

Re: [PATCH] D14871: [Power PC] fix calculating address of arguments on stack for variadic functions

2015-12-02 Thread Strahinja Petrovic via cfe-commits 
spetrovic updated this revision to Diff 41613.
spetrovic marked an inline comment as done.

http://reviews.llvm.org/D14871

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/ppc-varargs-struct.c
  test/CodeGen/x86_64-arguments.c

Index: test/CodeGen/x86_64-arguments.c
===
--- test/CodeGen/x86_64-arguments.c
+++ test/CodeGen/x86_64-arguments.c
@@ -336,7 +336,8 @@
 
 // CHECK-LABEL: define i32 @f44
 // CHECK: ptrtoint
-// CHECK-NEXT: and {{.*}}, -32
+// CHECK-NEXT: add i64 %{{[0-9]+}}, 31
+// CHECK-NEXT: and i64 %{{[0-9]+}}, -32
 // CHECK-NEXT: inttoptr
 typedef int T44 __attribute((vector_size(32)));
 struct s44 { T44 x; int y; };
Index: test/CodeGen/ppc-varargs-struct.c
===
--- test/CodeGen/ppc-varargs-struct.c
+++ test/CodeGen/ppc-varargs-struct.c
@@ -39,9 +39,13 @@
 // CHECK-PPC:[[USING_OVERFLOW]]
 // CHECK-PPC-NEXT:  [[OVERFLOW_AREA_P:%[0-9]+]] = getelementptr inbounds %struct.__va_list_tag, %struct.__va_list_tag* [[ARRAYDECAY]], i32 0, i32 3
 // CHECK-PPC-NEXT:  [[OVERFLOW_AREA:%.+]] = load i8*, i8** [[OVERFLOW_AREA_P]], align 4
-// CHECK-PPC-NEXT:  [[MEMADDR:%.+]] = bitcast i8* [[OVERFLOW_AREA]] to %struct.x**
-// CHECK-PPC-NEXT:  [[NEW_OVERFLOW_AREA:%[0-9]+]] = getelementptr inbounds i8, i8* [[OVERFLOW_AREA]], i32 4
-// CHECK-PPC-NEXT:  store i8* [[NEW_OVERFLOW_AREA]], i8** [[OVERFLOW_AREA_P]]
+// CHECK-PPC-NEXT:  %{{[0-9]+}} =  ptrtoint i8* %argp.cur to i32
+// CHECK-PPC-NEXT:  %{{[0-9]+}} = add i32 %{{[0-9]+}}, 7
+// CHECK-PPC-NEXT:  %{{[0-9]+}} = and i32 %{{[0-9]+}}, -8
+// CHECK-PPC-NEXT:  %argp.cur.aligned = inttoptr i32 %{{[0-9]+}} to i8*
+// CHECK-PPC-NEXT:  [[MEMADDR:%.+]] = bitcast i8* %argp.cur.aligned to %struct.x**
+// CHECK-PPC-NEXT:  [[NEW_OVERFLOW_AREA:%[0-9]+]] = getelementptr inbounds i8, i8* %argp.cur.aligned, i32 4
+// CHECK-PPC-NEXT:  store i8* [[NEW_OVERFLOW_AREA:%[0-9]+]], i8** [[OVERFLOW_AREA_P]], align 4
 // CHECK-PPC-NEXT:  br label %[[CONT]]
 //
 // CHECK-PPC:[[CONT]]
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -162,6 +162,23 @@
   OS << ")\n";
 }
 
+// Dynamically round a pointer up to a multiple of the given alignment.
+static llvm::Value *emitRoundPointerUpToAlignment(CodeGenFunction ,
+  llvm::Value *Ptr,
+  CharUnits Align) {
+  llvm::Value *PtrAsInt = Ptr;
+  // OverflowArgArea = (OverflowArgArea + Align - 1) & -Align;
+  PtrAsInt = CGF.Builder.CreatePtrToInt(PtrAsInt, CGF.IntPtrTy);
+  PtrAsInt = CGF.Builder.CreateAdd(PtrAsInt,
+llvm::ConstantInt::get(CGF.IntPtrTy, Align.getQuantity() - 1));
+  PtrAsInt = CGF.Builder.CreateAnd(PtrAsInt,
+   llvm::ConstantInt::get(CGF.IntPtrTy, -Align.getQuantity()));
+  PtrAsInt = CGF.Builder.CreateIntToPtr(PtrAsInt,
+Ptr->getType(),
+Ptr->getName() + ".aligned");
+  return PtrAsInt;
+}
+
 /// Emit va_arg for a platform using the common void* representation,
 /// where arguments are simply emitted in an array of slots on the stack.
 ///
@@ -193,17 +210,10 @@
   // If the CC aligns values higher than the slot size, do so if needed.
   Address Addr = Address::invalid();
   if (AllowHigherAlign && DirectAlign > SlotSize) {
-llvm::Value *PtrAsInt = Ptr;
-PtrAsInt = CGF.Builder.CreatePtrToInt(PtrAsInt, CGF.IntPtrTy);
-PtrAsInt = CGF.Builder.CreateAdd(PtrAsInt,
-  llvm::ConstantInt::get(CGF.IntPtrTy, DirectAlign.getQuantity() - 1));
-PtrAsInt = CGF.Builder.CreateAnd(PtrAsInt,
- llvm::ConstantInt::get(CGF.IntPtrTy, -DirectAlign.getQuantity()));
-Addr = Address(CGF.Builder.CreateIntToPtr(PtrAsInt, Ptr->getType(),
-  "argp.cur.aligned"),
-   DirectAlign);
+Addr = Address(emitRoundPointerUpToAlignment(CGF, Ptr, DirectAlign),
+ DirectAlign);
   } else {
-Addr = Address(Ptr, SlotSize);
+Addr = Address(Ptr, SlotSize); 
   }
 
   // Advance the pointer past the argument, then store that back.
@@ -3072,19 +3082,10 @@
   // byte boundary if alignment needed by type exceeds 8 byte boundary.
   // It isn't stated explicitly in the standard, but in practice we use
   // alignment greater than 16 where necessary.
-  uint64_t Align = CGF.getContext().getTypeAlignInChars(Ty).getQuantity();
-  if (Align > 8) {
-// overflow_arg_area = (overflow_arg_area + align - 1) & -align;
-llvm::Value *Offset =
-  llvm::ConstantInt::get(CGF.Int64Ty, Align - 1);
-overflow_arg_area = CGF.Builder.CreateGEP(overflow_arg_area, Offset);
-llvm::Value *AsInt = CGF.Builder.CreatePtrToInt(overflow_arg_area,
-

Re: [PATCH] D14871: [Power PC] fix calculating address of arguments on stack for variadic functions

2015-12-01 Thread John McCall via cfe-commits 
rjmccall added a comment.

Please feel free to commit after you make this last change.



Comment at: lib/CodeGen/TargetInfo.cpp:216
@@ +215,3 @@
+  }
+  else {
+Addr = Address(Ptr, SlotSize); 

LLVM code style is to put the else on the same line as the closing brace.


http://reviews.llvm.org/D14871



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D14871: [Power PC] fix calculating address of arguments on stack for variadic functions

2015-12-01 Thread Strahinja Petrovic via cfe-commits 
spetrovic updated this revision to Diff 41498.
spetrovic marked 3 inline comments as done.

http://reviews.llvm.org/D14871

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/ppc-varargs-struct.c
  test/CodeGen/x86_64-arguments.c

Index: test/CodeGen/x86_64-arguments.c
===
--- test/CodeGen/x86_64-arguments.c
+++ test/CodeGen/x86_64-arguments.c
@@ -336,7 +336,8 @@
 
 // CHECK-LABEL: define i32 @f44
 // CHECK: ptrtoint
-// CHECK-NEXT: and {{.*}}, -32
+// CHECK-NEXT: add i64 %{{[0-9]+}}, 31
+// CHECK-NEXT: and i64 %{{[0-9]+}}, -32
 // CHECK-NEXT: inttoptr
 typedef int T44 __attribute((vector_size(32)));
 struct s44 { T44 x; int y; };
Index: test/CodeGen/ppc-varargs-struct.c
===
--- test/CodeGen/ppc-varargs-struct.c
+++ test/CodeGen/ppc-varargs-struct.c
@@ -39,9 +39,13 @@
 // CHECK-PPC:[[USING_OVERFLOW]]
 // CHECK-PPC-NEXT:  [[OVERFLOW_AREA_P:%[0-9]+]] = getelementptr inbounds %struct.__va_list_tag, %struct.__va_list_tag* [[ARRAYDECAY]], i32 0, i32 3
 // CHECK-PPC-NEXT:  [[OVERFLOW_AREA:%.+]] = load i8*, i8** [[OVERFLOW_AREA_P]], align 4
-// CHECK-PPC-NEXT:  [[MEMADDR:%.+]] = bitcast i8* [[OVERFLOW_AREA]] to %struct.x**
-// CHECK-PPC-NEXT:  [[NEW_OVERFLOW_AREA:%[0-9]+]] = getelementptr inbounds i8, i8* [[OVERFLOW_AREA]], i32 4
-// CHECK-PPC-NEXT:  store i8* [[NEW_OVERFLOW_AREA]], i8** [[OVERFLOW_AREA_P]]
+// CHECK-PPC-NEXT:  %{{[0-9]+}} =  ptrtoint i8* %argp.cur to i32
+// CHECK-PPC-NEXT:  %{{[0-9]+}} = add i32 %{{[0-9]+}}, 7
+// CHECK-PPC-NEXT:  %{{[0-9]+}} = and i32 %{{[0-9]+}}, -8
+// CHECK-PPC-NEXT:  %argp.cur.aligned = inttoptr i32 %{{[0-9]+}} to i8*
+// CHECK-PPC-NEXT:  [[MEMADDR:%.+]] = bitcast i8* %argp.cur.aligned to %struct.x**
+// CHECK-PPC-NEXT:  [[NEW_OVERFLOW_AREA:%[0-9]+]] = getelementptr inbounds i8, i8* %argp.cur.aligned, i32 4
+// CHECK-PPC-NEXT:  store i8* [[NEW_OVERFLOW_AREA:%[0-9]+]], i8** [[OVERFLOW_AREA_P]], align 4
 // CHECK-PPC-NEXT:  br label %[[CONT]]
 //
 // CHECK-PPC:[[CONT]]
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -162,6 +162,23 @@
   OS << ")\n";
 }
 
+// Dynamically round a pointer up to a multiple of the given alignment.
+static llvm::Value *emitRoundPointerUpToAlignment(CodeGenFunction ,
+  llvm::Value *Ptr,
+  CharUnits Align) {
+  llvm::Value *PtrAsInt = Ptr;
+  // OverflowArgArea = (OverflowArgArea + Align - 1) & -Align;
+  PtrAsInt = CGF.Builder.CreatePtrToInt(PtrAsInt, CGF.IntPtrTy);
+  PtrAsInt = CGF.Builder.CreateAdd(PtrAsInt,
+llvm::ConstantInt::get(CGF.IntPtrTy, Align.getQuantity() - 1));
+  PtrAsInt = CGF.Builder.CreateAnd(PtrAsInt,
+   llvm::ConstantInt::get(CGF.IntPtrTy, -Align.getQuantity()));
+  PtrAsInt = CGF.Builder.CreateIntToPtr(PtrAsInt,
+Ptr->getType(),
+Ptr->getName() + ".aligned");
+  return PtrAsInt;
+}
+
 /// Emit va_arg for a platform using the common void* representation,
 /// where arguments are simply emitted in an array of slots on the stack.
 ///
@@ -193,17 +210,11 @@
   // If the CC aligns values higher than the slot size, do so if needed.
   Address Addr = Address::invalid();
   if (AllowHigherAlign && DirectAlign > SlotSize) {
-llvm::Value *PtrAsInt = Ptr;
-PtrAsInt = CGF.Builder.CreatePtrToInt(PtrAsInt, CGF.IntPtrTy);
-PtrAsInt = CGF.Builder.CreateAdd(PtrAsInt,
-  llvm::ConstantInt::get(CGF.IntPtrTy, DirectAlign.getQuantity() - 1));
-PtrAsInt = CGF.Builder.CreateAnd(PtrAsInt,
- llvm::ConstantInt::get(CGF.IntPtrTy, -DirectAlign.getQuantity()));
-Addr = Address(CGF.Builder.CreateIntToPtr(PtrAsInt, Ptr->getType(),
-  "argp.cur.aligned"),
-   DirectAlign);
-  } else {
-Addr = Address(Ptr, SlotSize);
+Addr = Address(emitRoundPointerUpToAlignment(CGF, Ptr, DirectAlign),
+ DirectAlign);
+  }
+  else {
+Addr = Address(Ptr, SlotSize); 
   }
 
   // Advance the pointer past the argument, then store that back.
@@ -3072,19 +3083,10 @@
   // byte boundary if alignment needed by type exceeds 8 byte boundary.
   // It isn't stated explicitly in the standard, but in practice we use
   // alignment greater than 16 where necessary.
-  uint64_t Align = CGF.getContext().getTypeAlignInChars(Ty).getQuantity();
-  if (Align > 8) {
-// overflow_arg_area = (overflow_arg_area + align - 1) & -align;
-llvm::Value *Offset =
-  llvm::ConstantInt::get(CGF.Int64Ty, Align - 1);
-overflow_arg_area = CGF.Builder.CreateGEP(overflow_arg_area, Offset);
-llvm::Value *AsInt = CGF.Builder.CreatePtrToInt(overflow_arg_area,
-

Re: [PATCH] D14871: [Power PC] fix calculating address of arguments on stack for variadic functions

2015-11-30 Thread John McCall via cfe-commits 
rjmccall added a comment.

Thank you.  A few more style complaints; with those fixed, LGTM.



Comment at: lib/CodeGen/TargetInfo.cpp:166
@@ +165,3 @@
+// Dynamically round a pointer up to a multiple of the given alignment.
+static llvm::Value* emitRoundPointerUpToAlignment(CodeGenFunction ,
+  llvm::Value *Ptr,

Minor code-style nitpick: please put the * next to the function name, not the 
type.


Comment at: lib/CodeGen/TargetInfo.cpp:216
@@ -208,1 +215,3 @@
+  else
+Addr = Address(Ptr, SlotSize); 
 

LLVM code style is to always leave the braces there.


Comment at: lib/CodeGen/TargetInfo.cpp:3087
@@ -3089,1 +3086,3 @@
+overflow_arg_area = emitRoundPointerUpToAlignment(CGF, overflow_arg_area,
+  Align);
 

Braces.


http://reviews.llvm.org/D14871



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D14871: [Power PC] fix calculating address of arguments on stack for variadic functions

2015-11-27 Thread Strahinja Petrovic via cfe-commits 
spetrovic updated this revision to Diff 41297.
spetrovic marked 2 inline comments as done.

http://reviews.llvm.org/D14871

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/ppc-varargs-struct.c
  test/CodeGen/x86_64-arguments.c

Index: test/CodeGen/x86_64-arguments.c
===
--- test/CodeGen/x86_64-arguments.c
+++ test/CodeGen/x86_64-arguments.c
@@ -336,7 +336,8 @@
 
 // CHECK-LABEL: define i32 @f44
 // CHECK: ptrtoint
-// CHECK-NEXT: and {{.*}}, -32
+// CHECK-NEXT: add i64 %{{[0-9]+}}, 31
+// CHECK-NEXT: and i64 %{{[0-9]+}}, -32
 // CHECK-NEXT: inttoptr
 typedef int T44 __attribute((vector_size(32)));
 struct s44 { T44 x; int y; };
Index: test/CodeGen/ppc-varargs-struct.c
===
--- test/CodeGen/ppc-varargs-struct.c
+++ test/CodeGen/ppc-varargs-struct.c
@@ -39,9 +39,13 @@
 // CHECK-PPC:[[USING_OVERFLOW]]
 // CHECK-PPC-NEXT:  [[OVERFLOW_AREA_P:%[0-9]+]] = getelementptr inbounds %struct.__va_list_tag, %struct.__va_list_tag* [[ARRAYDECAY]], i32 0, i32 3
 // CHECK-PPC-NEXT:  [[OVERFLOW_AREA:%.+]] = load i8*, i8** [[OVERFLOW_AREA_P]], align 4
-// CHECK-PPC-NEXT:  [[MEMADDR:%.+]] = bitcast i8* [[OVERFLOW_AREA]] to %struct.x**
-// CHECK-PPC-NEXT:  [[NEW_OVERFLOW_AREA:%[0-9]+]] = getelementptr inbounds i8, i8* [[OVERFLOW_AREA]], i32 4
-// CHECK-PPC-NEXT:  store i8* [[NEW_OVERFLOW_AREA]], i8** [[OVERFLOW_AREA_P]]
+// CHECK-PPC-NEXT:  %{{[0-9]+}} =  ptrtoint i8* %argp.cur to i32
+// CHECK-PPC-NEXT:  %{{[0-9]+}} = add i32 %{{[0-9]+}}, 7
+// CHECK-PPC-NEXT:  %{{[0-9]+}} = and i32 %{{[0-9]+}}, -8
+// CHECK-PPC-NEXT:  %argp.cur.aligned = inttoptr i32 %{{[0-9]+}} to i8*
+// CHECK-PPC-NEXT:  [[MEMADDR:%.+]] = bitcast i8* %argp.cur.aligned to %struct.x**
+// CHECK-PPC-NEXT:  [[NEW_OVERFLOW_AREA:%[0-9]+]] = getelementptr inbounds i8, i8* %argp.cur.aligned, i32 4
+// CHECK-PPC-NEXT:  store i8* [[NEW_OVERFLOW_AREA:%[0-9]+]], i8** [[OVERFLOW_AREA_P]], align 4
 // CHECK-PPC-NEXT:  br label %[[CONT]]
 //
 // CHECK-PPC:[[CONT]]
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -162,6 +162,23 @@
   OS << ")\n";
 }
 
+// Dynamically round a pointer up to a multiple of the given alignment.
+static llvm::Value* emitRoundPointerUpToAlignment(CodeGenFunction ,
+  llvm::Value *Ptr,
+  CharUnits Align) {
+  llvm::Value *PtrAsInt = Ptr;
+  // OverflowArgArea = (OverflowArgArea + Align - 1) & -Align;
+  PtrAsInt = CGF.Builder.CreatePtrToInt(PtrAsInt, CGF.IntPtrTy);
+  PtrAsInt = CGF.Builder.CreateAdd(PtrAsInt,
+llvm::ConstantInt::get(CGF.IntPtrTy, Align.getQuantity() - 1));
+  PtrAsInt = CGF.Builder.CreateAnd(PtrAsInt,
+   llvm::ConstantInt::get(CGF.IntPtrTy, -Align.getQuantity()));
+  PtrAsInt = CGF.Builder.CreateIntToPtr(PtrAsInt,
+Ptr->getType(),
+Ptr->getName() + ".aligned");
+  return PtrAsInt;
+}
+
 /// Emit va_arg for a platform using the common void* representation,
 /// where arguments are simply emitted in an array of slots on the stack.
 ///
@@ -192,19 +209,11 @@
 
   // If the CC aligns values higher than the slot size, do so if needed.
   Address Addr = Address::invalid();
-  if (AllowHigherAlign && DirectAlign > SlotSize) {
-llvm::Value *PtrAsInt = Ptr;
-PtrAsInt = CGF.Builder.CreatePtrToInt(PtrAsInt, CGF.IntPtrTy);
-PtrAsInt = CGF.Builder.CreateAdd(PtrAsInt,
-  llvm::ConstantInt::get(CGF.IntPtrTy, DirectAlign.getQuantity() - 1));
-PtrAsInt = CGF.Builder.CreateAnd(PtrAsInt,
- llvm::ConstantInt::get(CGF.IntPtrTy, -DirectAlign.getQuantity()));
-Addr = Address(CGF.Builder.CreateIntToPtr(PtrAsInt, Ptr->getType(),
-  "argp.cur.aligned"),
-   DirectAlign);
-  } else {
-Addr = Address(Ptr, SlotSize);
-  }
+  if (AllowHigherAlign && DirectAlign > SlotSize)
+Addr = Address(emitRoundPointerUpToAlignment(CGF, Ptr, DirectAlign),
+ DirectAlign);
+  else
+Addr = Address(Ptr, SlotSize); 
 
   // Advance the pointer past the argument, then store that back.
   CharUnits FullDirectSize = DirectSize.RoundUpToAlignment(SlotSize);
@@ -3072,20 +3081,10 @@
   // byte boundary if alignment needed by type exceeds 8 byte boundary.
   // It isn't stated explicitly in the standard, but in practice we use
   // alignment greater than 16 where necessary.
-  uint64_t Align = CGF.getContext().getTypeAlignInChars(Ty).getQuantity();
-  if (Align > 8) {
-// overflow_arg_area = (overflow_arg_area + align - 1) & -align;
-llvm::Value *Offset =
-  llvm::ConstantInt::get(CGF.Int64Ty, Align - 1);
-overflow_arg_area = CGF.Builder.CreateGEP(overflow_arg_area, Offset);
-

Re: [PATCH] D14871: [Power PC] fix calculating address of arguments on stack for variadic functions

2015-11-27 Thread Strahinja Petrovic via cfe-commits 
spetrovic added a comment.

Comments addressed.


http://reviews.llvm.org/D14871



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D14871: [Power PC] fix calculating address of arguments on stack for variadic functions

2015-11-25 Thread Strahinja Petrovic via cfe-commits 
spetrovic updated the summary for this revision.
spetrovic updated this revision to Diff 41142.
spetrovic marked 4 inline comments as done.

http://reviews.llvm.org/D14871

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/ppc-varargs-struct.c

Index: test/CodeGen/ppc-varargs-struct.c
===
--- test/CodeGen/ppc-varargs-struct.c
+++ test/CodeGen/ppc-varargs-struct.c
@@ -39,9 +39,13 @@
 // CHECK-PPC:[[USING_OVERFLOW]]
 // CHECK-PPC-NEXT:  [[OVERFLOW_AREA_P:%[0-9]+]] = getelementptr inbounds 
%struct.__va_list_tag, %struct.__va_list_tag* [[ARRAYDECAY]], i32 0, i32 3
 // CHECK-PPC-NEXT:  [[OVERFLOW_AREA:%.+]] = load i8*, i8** 
[[OVERFLOW_AREA_P]], align 4
-// CHECK-PPC-NEXT:  [[MEMADDR:%.+]] = bitcast i8* [[OVERFLOW_AREA]] to 
%struct.x**
-// CHECK-PPC-NEXT:  [[NEW_OVERFLOW_AREA:%[0-9]+]] = getelementptr inbounds i8, 
i8* [[OVERFLOW_AREA]], i32 4
-// CHECK-PPC-NEXT:  store i8* [[NEW_OVERFLOW_AREA]], i8** [[OVERFLOW_AREA_P]]
+// CHECK-PPC-NEXT:  %{{[0-9]+}} =  ptrtoint i8* %{{[0-9]+}} to i32
+// CHECK-PPC-NEXT:  %{{[0-9]+}} = add i32 %{{[0-9]+}}, 7
+// CHECK-PPC-NEXT:  %{{[0-9]+}} = and i32 %{{[0-9]+}}, -8
+// CHECK-PPC-NEXT:  %overflow_arg_area.align = inttoptr i32 %{{[0-9]+}} to i8*
+// CHECK-PPC-NEXT:  [[MEMADDR:%.+]] = bitcast i8* %overflow_arg_area.align to 
%struct.x**
+// CHECK-PPC-NEXT:  [[NEW_OVERFLOW_AREA:%[0-9]+]] = getelementptr inbounds i8, 
i8* %overflow_arg_area.align, i32 4
+// CHECK-PPC-NEXT:  store i8* [[NEW_OVERFLOW_AREA:%[0-9]+]], i8** 
[[OVERFLOW_AREA_P]], align 4
 // CHECK-PPC-NEXT:  br label %[[CONT]]
 //
 // CHECK-PPC:[[CONT]]
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -223,6 +223,23 @@
   return Addr;
 }
 
+static Address emitRoundPointerUpToAlignment(CodeGenFunction ,
+ llvm::Value *OverflowArgArea,
+ CharUnits Align) {
+  llvm::Value *PtrAsInt = OverflowArgArea;
+  // OverflowArgArea = (OverflowArgArea + Align - 1) & -Align;
+  PtrAsInt = CGF.Builder.CreatePtrToInt(PtrAsInt, CGF.IntPtrTy);
+  PtrAsInt = CGF.Builder.CreateAdd(PtrAsInt,
+llvm::ConstantInt::get(CGF.IntPtrTy, Align.getQuantity() - 1));
+  llvm::Value *Mask = llvm::ConstantInt::get(CGF.Int32Ty,
+ -Align.getQuantity());
+  PtrAsInt = CGF.Builder.CreateAnd(PtrAsInt, Mask);
+  PtrAsInt = CGF.Builder.CreateIntToPtr(PtrAsInt,
+OverflowArgArea->getType(),
+"overflow_arg_area.align");
+  return Address(PtrAsInt, Align);
+}
+
 /// Emit va_arg for a platform using the common void* representation,
 /// where arguments are simply emitted in an array of slots on the stack.
 ///
@@ -3543,9 +3560,14 @@
   Builder.CreateStructGEP(VAList, 3, CharUnits::fromQuantity(4));
 Address OverflowArea(Builder.CreateLoad(OverflowAreaAddr),
  OverflowAreaAlign);
-
-// The current address is the address of the varargs element.
-// FIXME: do we not need to round up to alignment?
+// Round up address of argument to alignment
+llvm::Value *OverflowArgArea = OverflowArea.getPointer();
+CharUnits Align = CGF.getContext().getTypeAlignInChars(Ty);
+if (Align > OverflowAreaAlign) {
+  OverflowArea = emitRoundPointerUpToAlignment(CGF, OverflowArgArea,
+   Align);
+}
+ 
 MemAddr = Builder.CreateElementBitCast(OverflowArea, DirectTy);
 
 // Increase the overflow area.


Index: test/CodeGen/ppc-varargs-struct.c
===
--- test/CodeGen/ppc-varargs-struct.c
+++ test/CodeGen/ppc-varargs-struct.c
@@ -39,9 +39,13 @@
 // CHECK-PPC:[[USING_OVERFLOW]]
 // CHECK-PPC-NEXT:  [[OVERFLOW_AREA_P:%[0-9]+]] = getelementptr inbounds %struct.__va_list_tag, %struct.__va_list_tag* [[ARRAYDECAY]], i32 0, i32 3
 // CHECK-PPC-NEXT:  [[OVERFLOW_AREA:%.+]] = load i8*, i8** [[OVERFLOW_AREA_P]], align 4
-// CHECK-PPC-NEXT:  [[MEMADDR:%.+]] = bitcast i8* [[OVERFLOW_AREA]] to %struct.x**
-// CHECK-PPC-NEXT:  [[NEW_OVERFLOW_AREA:%[0-9]+]] = getelementptr inbounds i8, i8* [[OVERFLOW_AREA]], i32 4
-// CHECK-PPC-NEXT:  store i8* [[NEW_OVERFLOW_AREA]], i8** [[OVERFLOW_AREA_P]]
+// CHECK-PPC-NEXT:  %{{[0-9]+}} =  ptrtoint i8* %{{[0-9]+}} to i32
+// CHECK-PPC-NEXT:  %{{[0-9]+}} = add i32 %{{[0-9]+}}, 7
+// CHECK-PPC-NEXT:  %{{[0-9]+}} = and i32 %{{[0-9]+}}, -8
+// CHECK-PPC-NEXT:  %overflow_arg_area.align = inttoptr i32 %{{[0-9]+}} to i8*
+// CHECK-PPC-NEXT:  [[MEMADDR:%.+]] = bitcast i8* %overflow_arg_area.align to %struct.x**
+// CHECK-PPC-NEXT:  [[NEW_OVERFLOW_AREA:%[0-9]+]] = getelementptr inbounds i8, i8* %overflow_arg_area.align, i32 4
+// CHECK-PPC-NEXT:  store i8* [[NEW_OVERFLOW_AREA:%[0-9]+]], i8**

Re: [PATCH] D14871: [Power PC] fix calculating address of arguments on stack for variadic functions

2015-11-25 Thread John McCall via cfe-commits 
rjmccall added inline comments.


Comment at: lib/CodeGen/TargetInfo.cpp:241
@@ +240,3 @@
+  return Address(PtrAsInt, Align);
+}
+

Thank you for extracting this.

First, this function deserves a doc comment now; I would suggest:

  /// Dynamically round a pointer up to a multiple of the given alignment.

Also, this is now generically useful, so (1) please rename OverflowArgArea to 
something more generic, like Ptr, and (2) please use LLVM value names based on 
the name already in Ptr.  That is, instead of "overflow_arg_area.align", please 
use
  Ptr->getName () + ".aligned"

Finally, please look for the other places in this file that you could change to 
use this new function.  There's one in emitVoidPtrDirectVAArg, and there are 
several other in other targets.


Comment at: lib/CodeGen/TargetInfo.cpp:3568
@@ +3567,3 @@
+  OverflowArea = emitRoundPointerUpToAlignment(CGF, OverflowArgArea,
+   Align);
+}

Please sink OverflowArgArea into the if block, since you don't need it outside.


http://reviews.llvm.org/D14871



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D14871: [Power PC] fix calculating address of arguments on stack for variadic functions

2015-11-23 Thread Strahinja Petrovic via cfe-commits 
spetrovic updated this revision to Diff 40915.
spetrovic marked an inline comment as done.

http://reviews.llvm.org/D14871

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/ppc-varargs-struct.c

Index: test/CodeGen/ppc-varargs-struct.c
===
--- test/CodeGen/ppc-varargs-struct.c
+++ test/CodeGen/ppc-varargs-struct.c
@@ -39,9 +39,13 @@
 // CHECK-PPC:[[USING_OVERFLOW]]
 // CHECK-PPC-NEXT:  [[OVERFLOW_AREA_P:%[0-9]+]] = getelementptr inbounds 
%struct.__va_list_tag, %struct.__va_list_tag* [[ARRAYDECAY]], i32 0, i32 3
 // CHECK-PPC-NEXT:  [[OVERFLOW_AREA:%.+]] = load i8*, i8** 
[[OVERFLOW_AREA_P]], align 4
-// CHECK-PPC-NEXT:  [[MEMADDR:%.+]] = bitcast i8* [[OVERFLOW_AREA]] to 
%struct.x**
-// CHECK-PPC-NEXT:  [[NEW_OVERFLOW_AREA:%[0-9]+]] = getelementptr inbounds i8, 
i8* [[OVERFLOW_AREA]], i32 4
-// CHECK-PPC-NEXT:  store i8* [[NEW_OVERFLOW_AREA]], i8** [[OVERFLOW_AREA_P]]
+// CHECK-PPC-NEXT:  %{{[0-9]+}} = getelementptr i8, i8* %{{[0-9]+}}, i32 7
+// CHECK-PPC-NEXT:  %{{[0-9]+}} = ptrtoint i8* %{{[0-9]+}} to i32
+// CHECK-PPC-NEXT:  %{{[0-9]+}} = and i32 %{{[0-9]+}}, -8
+// CHECK-PPC-NEXT:  %overflow_arg_area.align = inttoptr i32 %{{[0-9]+}} to i8*
+// CHECK-PPC-NEXT:  [[MEMADDR:%.+]] = bitcast i8* %overflow_arg_area.align to 
%struct.x**
+// CHECK-PPC-NEXT:  [[NEW_OVERFLOW_AREA:%[0-9]+]] = getelementptr inbounds i8, 
i8* %overflow_arg_area.align, i32 4
+// CHECK-PPC-NEXT:  store i8* [[NEW_OVERFLOW_AREA:%[0-9]+]], i8** 
[[OVERFLOW_AREA_P]], align 4
 // CHECK-PPC-NEXT:  br label %[[CONT]]
 //
 // CHECK-PPC:[[CONT]]
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -3543,9 +3543,23 @@
   Builder.CreateStructGEP(VAList, 3, CharUnits::fromQuantity(4));
 Address OverflowArea(Builder.CreateLoad(OverflowAreaAddr),
  OverflowAreaAlign);
-
-// The current address is the address of the varargs element.
-// FIXME: do we not need to round up to alignment?
+// Round up address of argument to alignment
+llvm::Value *OverflowArgArea = OverflowArea.getPointer();
+uint32_t Align = CGF.getContext().getTypeAlignInChars(Ty).getQuantity();
+if (Align > 4) {
+  // OverflowArgArea = (OverflowArgArea + Align - 1) & -Align;
+  llvm::Value *Offset = llvm::ConstantInt::get(CGF.Int32Ty, Align - 1);
+  OverflowArgArea = CGF.Builder.CreateGEP(OverflowArgArea, Offset);
+  llvm::Value *AsInt = CGF.Builder.CreatePtrToInt(OverflowArgArea,
+  CGF.Int32Ty);
+  llvm::Value *Mask = llvm::ConstantInt::get(CGF.Int32Ty, 
-(uint32_t)Align);
+  OverflowArgArea =
+CGF.Builder.CreateIntToPtr(CGF.Builder.CreateAnd(AsInt, Mask),
+   OverflowArgArea->getType(),
+   "overflow_arg_area.align");
+}
+ 
+OverflowArea = Address(OverflowArgArea, CharUnits::fromQuantity(Align));
 MemAddr = Builder.CreateElementBitCast(OverflowArea, DirectTy);
 
 // Increase the overflow area.


Index: test/CodeGen/ppc-varargs-struct.c
===
--- test/CodeGen/ppc-varargs-struct.c
+++ test/CodeGen/ppc-varargs-struct.c
@@ -39,9 +39,13 @@
 // CHECK-PPC:[[USING_OVERFLOW]]
 // CHECK-PPC-NEXT:  [[OVERFLOW_AREA_P:%[0-9]+]] = getelementptr inbounds %struct.__va_list_tag, %struct.__va_list_tag* [[ARRAYDECAY]], i32 0, i32 3
 // CHECK-PPC-NEXT:  [[OVERFLOW_AREA:%.+]] = load i8*, i8** [[OVERFLOW_AREA_P]], align 4
-// CHECK-PPC-NEXT:  [[MEMADDR:%.+]] = bitcast i8* [[OVERFLOW_AREA]] to %struct.x**
-// CHECK-PPC-NEXT:  [[NEW_OVERFLOW_AREA:%[0-9]+]] = getelementptr inbounds i8, i8* [[OVERFLOW_AREA]], i32 4
-// CHECK-PPC-NEXT:  store i8* [[NEW_OVERFLOW_AREA]], i8** [[OVERFLOW_AREA_P]]
+// CHECK-PPC-NEXT:  %{{[0-9]+}} = getelementptr i8, i8* %{{[0-9]+}}, i32 7
+// CHECK-PPC-NEXT:  %{{[0-9]+}} = ptrtoint i8* %{{[0-9]+}} to i32
+// CHECK-PPC-NEXT:  %{{[0-9]+}} = and i32 %{{[0-9]+}}, -8
+// CHECK-PPC-NEXT:  %overflow_arg_area.align = inttoptr i32 %{{[0-9]+}} to i8*
+// CHECK-PPC-NEXT:  [[MEMADDR:%.+]] = bitcast i8* %overflow_arg_area.align to %struct.x**
+// CHECK-PPC-NEXT:  [[NEW_OVERFLOW_AREA:%[0-9]+]] = getelementptr inbounds i8, i8* %overflow_arg_area.align, i32 4
+// CHECK-PPC-NEXT:  store i8* [[NEW_OVERFLOW_AREA:%[0-9]+]], i8** [[OVERFLOW_AREA_P]], align 4
 // CHECK-PPC-NEXT:  br label %[[CONT]]
 //
 // CHECK-PPC:[[CONT]]
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -3543,9 +3543,23 @@
   Builder.CreateStructGEP(VAList, 3, CharUnits::fromQuantity(4));
 Address OverflowArea(Builder.CreateLoad(OverflowAreaAddr),
  OverflowAreaAlign);
-
-// The current address is the address of the

Re: [PATCH] D14871: [Power PC] fix calculating address of arguments on stack for variadic functions

2015-11-23 Thread John McCall via cfe-commits 
rjmccall added inline comments.


Comment at: lib/CodeGen/TargetInfo.cpp:3548
@@ +3547,3 @@
+llvm::Value *OverflowArgArea = OverflowArea.getPointer();
+uint32_t Align = CGF.getContext().getTypeAlignInChars(Ty).getQuantity();
+if (Align > 4) {

This patch applies to more types than just double.  You should probably add 
tests for some of the other cases, but if not, please at least make your commit 
message convey the intended generality.


Comment at: lib/CodeGen/TargetInfo.cpp:3549
@@ +3548,3 @@
+uint32_t Align = CGF.getContext().getTypeAlignInChars(Ty).getQuantity();
+if (Align > 4) {
+  // OverflowArgArea = (OverflowArgArea + Align - 1) & -Align;

if (Align > OverflowAreaAlign)


Comment at: lib/CodeGen/TargetInfo.cpp:3559
@@ +3558,3 @@
+   OverflowArgArea->getType(),
+   "overflow_arg_area.align");
+}

Would you mind extracting a function to do this realignment?  There are 3-4 
other places in this file that do exactly this.  Using the existing code 
pattern from emitVoidPtrDirectVAArg — i.e. using an add instead of a 
non-inbounds GEP — seems reasonable.

I think this is a sensible prototype:
  static Address emitRoundPointerUpToAlignment(CodeGenFunction , 
llvm::Value *Ptr, CharUnits Align, const llvm::Twine );


Comment at: lib/CodeGen/TargetInfo.cpp:3562
@@ -3549,1 +3561,3 @@
+ 
+OverflowArea = Address(OverflowArgArea, CharUnits::fromQuantity(Align));
 MemAddr = Builder.CreateElementBitCast(OverflowArea, DirectTy);

This code will be more readable if you keep Align as a CharUnits.


http://reviews.llvm.org/D14871



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D14871: [Power PC] fix calculating address of arguments on stack for variadic functions

2015-11-21 Thread hfin...@anl.gov via cfe-commits 
hfinkel added inline comments.


Comment at: lib/CodeGen/TargetInfo.cpp:3547
@@ +3546,3 @@
+// Round up address of argument to alignment
+llvm::Value *overflow_arg_area = OverflowArea.getPointer();
+uint32_t Align = CGF.getContext().getTypeAlignInChars(Ty).getQuantity();

You need to use variable names consistent with our coding convention 
(OverflowArgArea, etc.).


http://reviews.llvm.org/D14871



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits