[PATCH] D60548: Variable auto-init: also auto-init alloca

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

I got an lgtm from @pcc on IRC, so I'll commit this.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60548



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

[PATCH] D60548: Variable auto-init: also auto-init alloca

[JF Bastien via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC358243: Variable auto-init: also auto-init alloca (authored 
by jfb, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60548?vs=194762=194787#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D60548

Files:
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CMakeLists.txt
  lib/CodeGen/PatternInit.cpp
  lib/CodeGen/PatternInit.h
  test/CodeGenCXX/trivial-auto-var-init.cpp

Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -19,6 +19,7 @@
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "ConstantEmitter.h"
+#include "PatternInit.h"
 #include "TargetInfo.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/CharUnits.h"
@@ -984,92 +985,13 @@
   return false;
 }
 
-static llvm::Constant *patternFor(CodeGenModule , llvm::Type *Ty) {
-  // The following value is a guaranteed unmappable pointer value and has a
-  // repeated byte-pattern which makes it easier to synthesize. We use it for
-  // pointers as well as integers so that aggregates are likely to be
-  // initialized with this repeated value.
-  constexpr uint64_t LargeValue = 0xull;
-  // For 32-bit platforms it's a bit trickier because, across systems, only the
-  // zero page can reasonably be expected to be unmapped, and even then we need
-  // a very low address. We use a smaller value, and that value sadly doesn't
-  // have a repeated byte-pattern. We don't use it for integers.
-  constexpr uint32_t SmallValue = 0x00AA;
-  // Floating-point values are initialized as NaNs because they propagate. Using
-  // a repeated byte pattern means that it will be easier to initialize
-  // all-floating-point aggregates and arrays with memset. Further, aggregates
-  // which mix integral and a few floats might also initialize with memset
-  // followed by a handful of stores for the floats. Using fairly unique NaNs
-  // also means they'll be easier to distinguish in a crash.
-  constexpr bool NegativeNaN = true;
-  constexpr uint64_t NaNPayload = 0xull;
-  if (Ty->isIntOrIntVectorTy()) {
-unsigned BitWidth = cast(
-Ty->isVectorTy() ? Ty->getVectorElementType() : Ty)
-->getBitWidth();
-if (BitWidth <= 64)
-  return llvm::ConstantInt::get(Ty, LargeValue);
-return llvm::ConstantInt::get(
-Ty, llvm::APInt::getSplat(BitWidth, llvm::APInt(64, LargeValue)));
-  }
-  if (Ty->isPtrOrPtrVectorTy()) {
-auto *PtrTy = cast(
-Ty->isVectorTy() ? Ty->getVectorElementType() : Ty);
-unsigned PtrWidth = CGM.getContext().getTargetInfo().getPointerWidth(
-PtrTy->getAddressSpace());
-llvm::Type *IntTy = llvm::IntegerType::get(CGM.getLLVMContext(), PtrWidth);
-uint64_t IntValue;
-switch (PtrWidth) {
-default:
-  llvm_unreachable("pattern initialization of unsupported pointer width");
-case 64:
-  IntValue = LargeValue;
-  break;
-case 32:
-  IntValue = SmallValue;
-  break;
-}
-auto *Int = llvm::ConstantInt::get(IntTy, IntValue);
-return llvm::ConstantExpr::getIntToPtr(Int, PtrTy);
-  }
-  if (Ty->isFPOrFPVectorTy()) {
-unsigned BitWidth = llvm::APFloat::semanticsSizeInBits(
-(Ty->isVectorTy() ? Ty->getVectorElementType() : Ty)
-->getFltSemantics());
-llvm::APInt Payload(64, NaNPayload);
-if (BitWidth >= 64)
-  Payload = llvm::APInt::getSplat(BitWidth, Payload);
-return llvm::ConstantFP::getQNaN(Ty, NegativeNaN, );
-  }
-  if (Ty->isArrayTy()) {
-// Note: this doesn't touch tail padding (at the end of an object, before
-// the next array object). It is instead handled by replaceUndef.
-auto *ArrTy = cast(Ty);
-llvm::SmallVector Element(
-ArrTy->getNumElements(), patternFor(CGM, ArrTy->getElementType()));
-return llvm::ConstantArray::get(ArrTy, Element);
-  }
-
-  // Note: this doesn't touch struct padding. It will initialize as much union
-  // padding as is required for the largest type in the union. Padding is
-  // instead handled by replaceUndef. Stores to structs with volatile members
-  // don't have a volatile qualifier when initialized according to C++. This is
-  // fine because stack-based volatiles don't really have volatile semantics
-  // anyways, and the initialization shouldn't be observable.
-  auto *StructTy = cast(Ty);
-  llvm::SmallVector Struct(StructTy->getNumElements());
-  for (unsigned El = 0; El != Struct.size(); ++El)
-Struct[El] = patternFor(CGM, StructTy->getElementType(El));
-  return llvm::ConstantStruct::get(StructTy, Struct);
-}
-
 enum class IsPattern { No, Yes };
 
 /// Generate a constant filled with either a pattern or zeroes.
 static llvm::Constant 

[PATCH] D60548: Variable auto-init: also auto-init alloca

[JF Bastien via Phabricator via cfe-commits]

jfb marked an inline comment as done.
jfb added inline comments.



Comment at: lib/CodeGen/PatternInit.h:22
+
+llvm::Constant *patternFor(CodeGenModule&, llvm::Type*);
+

rjmccall wrote:
> jfb wrote:
> > rjmccall wrote:
> > > Please choose names that mean something outside of the mental context you 
> > > were in when you wrote the patch. :)
> > I was just copy / pasting!
> Your own code. :)
Hey you can't pull that one on me! Pulling it on you is *my* hobby!
And like... it's communal code now! ;-)


Repository:
  rC Clang

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

https://reviews.llvm.org/D60548



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

[PATCH] D60548: Variable auto-init: also auto-init alloca

[John McCall via Phabricator via cfe-commits]

rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

LGTM.




Comment at: lib/CodeGen/PatternInit.h:22
+
+llvm::Constant *patternFor(CodeGenModule&, llvm::Type*);
+

jfb wrote:
> rjmccall wrote:
> > Please choose names that mean something outside of the mental context you 
> > were in when you wrote the patch. :)
> I was just copy / pasting!
Your own code. :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D60548



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

[PATCH] D60548: Variable auto-init: also auto-init alloca

[JF Bastien via Phabricator via cfe-commits]

jfb marked 2 inline comments as done.
jfb added inline comments.



Comment at: lib/CodeGen/PatternInit.h:22
+
+llvm::Constant *patternFor(CodeGenModule&, llvm::Type*);
+

rjmccall wrote:
> Please choose names that mean something outside of the mental context you 
> were in when you wrote the patch. :)
I was just copy / pasting!


Repository:
  rC Clang

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

https://reviews.llvm.org/D60548



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

[PATCH] D60548: Variable auto-init: also auto-init alloca

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 194762.
jfb marked 2 inline comments as done.
jfb added a comment.

- Change name, qualify declaration.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60548

Files:
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CMakeLists.txt
  lib/CodeGen/PatternInit.cpp
  lib/CodeGen/PatternInit.h
  test/CodeGenCXX/trivial-auto-var-init.cpp

Index: test/CodeGenCXX/trivial-auto-var-init.cpp
===
--- test/CodeGenCXX/trivial-auto-var-init.cpp
+++ test/CodeGenCXX/trivial-auto-var-init.cpp
@@ -173,6 +173,34 @@
   used(ptr);
 }
 
+// UNINIT-LABEL:  test_alloca(
+// ZERO-LABEL:test_alloca(
+// ZERO:  %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// ZERO-NEXT: %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align [[ALIGN:[0-9]+]]
+// ZERO-NEXT: call void @llvm.memset{{.*}}(i8* align [[ALIGN]] %[[ALLOCA]], i8 0, i64 %[[SIZE]], i1 false)
+// PATTERN-LABEL: test_alloca(
+// PATTERN:   %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// PATTERN-NEXT:  %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align [[ALIGN:[0-9]+]]
+// PATTERN-NEXT:  call void @llvm.memset{{.*}}(i8* align [[ALIGN]] %[[ALLOCA]], i8 -86, i64 %[[SIZE]], i1 false)
+void test_alloca(int size) {
+  void *ptr = __builtin_alloca(size);
+  used(ptr);
+}
+
+// UNINIT-LABEL:  test_alloca_with_align(
+// ZERO-LABEL:test_alloca_with_align(
+// ZERO:  %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// ZERO-NEXT: %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align 128
+// ZERO-NEXT: call void @llvm.memset{{.*}}(i8* align 128 %[[ALLOCA]], i8 0, i64 %[[SIZE]], i1 false)
+// PATTERN-LABEL: test_alloca_with_align(
+// PATTERN:   %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// PATTERN-NEXT:  %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align 128
+// PATTERN-NEXT:  call void @llvm.memset{{.*}}(i8* align 128 %[[ALLOCA]], i8 -86, i64 %[[SIZE]], i1 false)
+void test_alloca_with_align(int size) {
+  void *ptr = __builtin_alloca_with_align(size, 1024);
+  used(ptr);
+}
+
 // UNINIT-LABEL:  test_struct_vla(
 // ZERO-LABEL:test_struct_vla(
 // ZERO:  %[[SIZE:[0-9]+]] = mul nuw i64 %{{.*}}, 16
Index: lib/CodeGen/PatternInit.h
===
--- /dev/null
+++ lib/CodeGen/PatternInit.h
@@ -0,0 +1,27 @@
+//===- PatternInit - Pattern initialization -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_CODEGEN_PATTERNINIT_H
+#define LLVM_CLANG_LIB_CODEGEN_PATTERNINIT_H
+
+namespace llvm {
+class Constant;
+class Type;
+} // namespace llvm
+
+namespace clang {
+namespace CodeGen {
+
+class CodeGenModule;
+
+llvm::Constant *initializationPatternFor(CodeGenModule &, llvm::Type *);
+
+} // end namespace CodeGen
+} // end namespace clang
+
+#endif
Index: lib/CodeGen/PatternInit.cpp
===
--- /dev/null
+++ lib/CodeGen/PatternInit.cpp
@@ -0,0 +1,93 @@
+//===--- PatternInit.cpp - Pattern Initialization -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "PatternInit.h"
+#include "CodeGenModule.h"
+#include "llvm/IR/Constant.h"
+#include "llvm/IR/Type.h"
+
+llvm::Constant *clang::CodeGen::initializationPatternFor(CodeGenModule ,
+ llvm::Type *Ty) {
+  // The following value is a guaranteed unmappable pointer value and has a
+  // repeated byte-pattern which makes it easier to synthesize. We use it for
+  // pointers as well as integers so that aggregates are likely to be
+  // initialized with this repeated value.
+  constexpr uint64_t LargeValue = 0xull;
+  // For 32-bit platforms it's a bit trickier because, across systems, only the
+  // zero page can reasonably be expected to be unmapped, and even then we need
+  // a very low address. We use a smaller value, and that value sadly doesn't
+  // have a repeated byte-pattern. We don't use it for integers.
+  constexpr uint32_t SmallValue = 0x00AA;
+  // Floating-point values are initialized as NaNs because they propagate. Using
+  // a repeated byte pattern means that it will be easier to initialize
+  // all-floating-point aggregates and arrays with memset. Further, aggregates
+  // which mix integral and a few floats might also 

[PATCH] D60548: Variable auto-init: also auto-init alloca

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/PatternInit.cpp:17
+
+llvm::Constant *patternFor(CodeGenModule , llvm::Type *Ty) {
+  // The following value is a guaranteed unmappable pointer value and has a

Please use a qualified declaration here instead of defining it in the namespace.



Comment at: lib/CodeGen/PatternInit.h:22
+
+llvm::Constant *patternFor(CodeGenModule&, llvm::Type*);
+

Please choose names that mean something outside of the mental context you were 
in when you wrote the patch. :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D60548



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

[PATCH] D60548: Variable auto-init: also auto-init alloca

[JF Bastien via Phabricator via cfe-commits]

jfb marked 2 inline comments as done.
jfb added inline comments.



Comment at: lib/CodeGen/CGBuiltin.cpp:57
+  case LangOptions::TrivialAutoVarInitKind::Pattern:
+Byte = CGF.Builder.getInt8(0xAA);
+break;

rjmccall wrote:
> Can this value be taken from some common source with the existing code?  I 
> mean, even if it's just a constant in some header, that'd be a start.
Done.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60548



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

[PATCH] D60548: Variable auto-init: also auto-init alloca

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 194736.
jfb added a comment.
Herald added a subscriber: mgorny.

- Move patternFor to a shared file as requested by @rjmccall


Repository:
  rC Clang

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

https://reviews.llvm.org/D60548

Files:
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CMakeLists.txt
  lib/CodeGen/PatternInit.cpp
  lib/CodeGen/PatternInit.h
  test/CodeGenCXX/trivial-auto-var-init.cpp

Index: test/CodeGenCXX/trivial-auto-var-init.cpp
===
--- test/CodeGenCXX/trivial-auto-var-init.cpp
+++ test/CodeGenCXX/trivial-auto-var-init.cpp
@@ -173,6 +173,34 @@
   used(ptr);
 }
 
+// UNINIT-LABEL:  test_alloca(
+// ZERO-LABEL:test_alloca(
+// ZERO:  %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// ZERO-NEXT: %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align [[ALIGN:[0-9]+]]
+// ZERO-NEXT: call void @llvm.memset{{.*}}(i8* align [[ALIGN]] %[[ALLOCA]], i8 0, i64 %[[SIZE]], i1 false)
+// PATTERN-LABEL: test_alloca(
+// PATTERN:   %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// PATTERN-NEXT:  %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align [[ALIGN:[0-9]+]]
+// PATTERN-NEXT:  call void @llvm.memset{{.*}}(i8* align [[ALIGN]] %[[ALLOCA]], i8 -86, i64 %[[SIZE]], i1 false)
+void test_alloca(int size) {
+  void *ptr = __builtin_alloca(size);
+  used(ptr);
+}
+
+// UNINIT-LABEL:  test_alloca_with_align(
+// ZERO-LABEL:test_alloca_with_align(
+// ZERO:  %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// ZERO-NEXT: %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align 128
+// ZERO-NEXT: call void @llvm.memset{{.*}}(i8* align 128 %[[ALLOCA]], i8 0, i64 %[[SIZE]], i1 false)
+// PATTERN-LABEL: test_alloca_with_align(
+// PATTERN:   %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// PATTERN-NEXT:  %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align 128
+// PATTERN-NEXT:  call void @llvm.memset{{.*}}(i8* align 128 %[[ALLOCA]], i8 -86, i64 %[[SIZE]], i1 false)
+void test_alloca_with_align(int size) {
+  void *ptr = __builtin_alloca_with_align(size, 1024);
+  used(ptr);
+}
+
 // UNINIT-LABEL:  test_struct_vla(
 // ZERO-LABEL:test_struct_vla(
 // ZERO:  %[[SIZE:[0-9]+]] = mul nuw i64 %{{.*}}, 16
Index: lib/CodeGen/PatternInit.h
===
--- /dev/null
+++ lib/CodeGen/PatternInit.h
@@ -0,0 +1,27 @@
+//===- PatternInit - Pattern initialization -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_CODEGEN_PATTERNINIT_H
+#define LLVM_CLANG_LIB_CODEGEN_PATTERNINIT_H
+
+namespace llvm {
+class Constant;
+class Type;
+}
+
+namespace clang {
+namespace CodeGen {
+
+class CodeGenModule;
+
+llvm::Constant *patternFor(CodeGenModule&, llvm::Type*);
+
+}  // end namespace CodeGen
+}  // end namespace clang
+
+#endif
Index: lib/CodeGen/PatternInit.cpp
===
--- /dev/null
+++ lib/CodeGen/PatternInit.cpp
@@ -0,0 +1,97 @@
+//===--- CGDecl.cpp - Emit LLVM Code for declarations -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "CodeGenModule.h"
+#include "PatternInit.h"
+#include "llvm/IR/Constant.h"
+#include "llvm/IR/Type.h"
+
+namespace clang {
+namespace CodeGen {
+
+llvm::Constant *patternFor(CodeGenModule , llvm::Type *Ty) {
+  // The following value is a guaranteed unmappable pointer value and has a
+  // repeated byte-pattern which makes it easier to synthesize. We use it for
+  // pointers as well as integers so that aggregates are likely to be
+  // initialized with this repeated value.
+  constexpr uint64_t LargeValue = 0xull;
+  // For 32-bit platforms it's a bit trickier because, across systems, only the
+  // zero page can reasonably be expected to be unmapped, and even then we need
+  // a very low address. We use a smaller value, and that value sadly doesn't
+  // have a repeated byte-pattern. We don't use it for integers.
+  constexpr uint32_t SmallValue = 0x00AA;
+  // Floating-point values are initialized as NaNs because they propagate. Using
+  // a repeated byte pattern means that it will be easier to initialize
+  // all-floating-point aggregates and arrays with memset. Further, aggregates
+  // which mix integral and a few floats might also initialize with memset
+  // followed by a handful of stores 

[PATCH] D60548: Variable auto-init: also auto-init alloca

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/CGBuiltin.cpp:57
+  case LangOptions::TrivialAutoVarInitKind::Pattern:
+Byte = CGF.Builder.getInt8(0xAA);
+break;

Can this value be taken from some common source with the existing code?  I 
mean, even if it's just a constant in some header, that'd be a start.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60548



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

[PATCH] D60548: Variable auto-init: also auto-init alloca

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D60548#1462124 , @jfb wrote:

> One downside to alloca is that we can's use `__attribute__((uninitialized))` 
> because it's a builtin. Maybe we need a separate uninitialized alloca? What 
> do you all think?


Actually I'm wondering if we should just implement:

  #pragma clang attribute push (__attribute__((uninitialized)), apply_to = 
variable(unless(is_global)))

And lean on that for `alloca`, instead of having a new uninitialized `alloca` 
builtin. The pragma is useful for debugging regardless, so I think overall it's 
better.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60548



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

[PATCH] D60548: Variable auto-init: also auto-init alloca

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D60548#1463181 , @jfb wrote:

> In D60548#1462124 , @jfb wrote:
>
> > One downside to alloca is that we can's use 
> > `__attribute__((uninitialized))` because it's a builtin. Maybe we need a 
> > separate uninitialized alloca? What do you all think?
>
>
> Actually I'm wondering if we should just implement:
>
>   #pragma clang attribute push (__attribute__((uninitialized)), apply_to = 
> variable(unless(is_global)))
>
>
> And lean on that for `alloca`, instead of having a new uninitialized `alloca` 
> builtin. The pragma is useful for debugging regardless, so I think overall 
> it's better.


Although the pragma syntax 

 doesn't seem to support builtins, only variables, so it's still; weird for 
alloca... I'd really want the pragma to apply to everything in its scope 
including alloca.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60548



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

[PATCH] D60548: Variable auto-init: also auto-init alloca

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

I probably wouldn't do anything about suppressing init on alloca for now, but 
if we did do something I think I'd be in favour of the separate builtin for 
uninitialized alloca. I also considered the alternative of allowing 
`__attribute__((uninitialized))` to appear on a function or block statement, 
but it seemed clearer for the uninitialized-ness to be as close to the alloca 
as possible.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60548



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

[PATCH] D60548: Variable auto-init: also auto-init alloca

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

One downside to alloca is that we can's use `__attribute__((uninitialized))` 
because it's a builtin. Maybe we need a separate uninitialized alloca? What do 
you all think?


Repository:
  rC Clang

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

https://reviews.llvm.org/D60548



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

[PATCH] D60548: Variable auto-init: also auto-init alloca

[JF Bastien via Phabricator via cfe-commits]

jfb created this revision.
Herald added subscribers: cfe-commits, dexonsmith, jkorous.
Herald added a project: clang.

alloca isn’t auto-init’d right now because it’s a different path in clang that 
all the other stuff we support (it’s a builtin, not an expression). 
Interestingly, alloca doesn’t have a type (as opposed to even VLA) so we can 
really only initialize it with memset.

rdar://problem/49794007


Repository:
  rC Clang

https://reviews.llvm.org/D60548

Files:
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGenCXX/trivial-auto-var-init.cpp


Index: test/CodeGenCXX/trivial-auto-var-init.cpp
===
--- test/CodeGenCXX/trivial-auto-var-init.cpp
+++ test/CodeGenCXX/trivial-auto-var-init.cpp
@@ -173,6 +173,34 @@
   used(ptr);
 }
 
+// UNINIT-LABEL:  test_alloca(
+// ZERO-LABEL:test_alloca(
+// ZERO:  %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// ZERO-NEXT: %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align 
[[ALIGN:[0-9]+]]
+// ZERO-NEXT: call void @llvm.memset{{.*}}(i8* align [[ALIGN]] 
%[[ALLOCA]], i8 0, i64 %[[SIZE]], i1 false)
+// PATTERN-LABEL: test_alloca(
+// PATTERN:   %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// PATTERN-NEXT:  %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align 
[[ALIGN:[0-9]+]]
+// PATTERN-NEXT:  call void @llvm.memset{{.*}}(i8* align [[ALIGN]] 
%[[ALLOCA]], i8 -86, i64 %[[SIZE]], i1 false)
+void test_alloca(int size) {
+  void *ptr = __builtin_alloca(size);
+  used(ptr);
+}
+
+// UNINIT-LABEL:  test_alloca_with_align(
+// ZERO-LABEL:test_alloca_with_align(
+// ZERO:  %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// ZERO-NEXT: %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align 128
+// ZERO-NEXT: call void @llvm.memset{{.*}}(i8* align 128 %[[ALLOCA]], i8 
0, i64 %[[SIZE]], i1 false)
+// PATTERN-LABEL: test_alloca_with_align(
+// PATTERN:   %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// PATTERN-NEXT:  %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align 128
+// PATTERN-NEXT:  call void @llvm.memset{{.*}}(i8* align 128 %[[ALLOCA]], i8 
-86, i64 %[[SIZE]], i1 false)
+void test_alloca_with_align(int size) {
+  void *ptr = __builtin_alloca_with_align(size, 1024);
+  used(ptr);
+}
+
 // UNINIT-LABEL:  test_struct_vla(
 // ZERO-LABEL:test_struct_vla(
 // ZERO:  %[[SIZE:[0-9]+]] = mul nuw i64 %{{.*}}, 16
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -44,6 +44,22 @@
   return std::min(High, std::max(Low, Value));
 }
 
+static void initializeAlloca(CodeGenFunction , AllocaInst *AI, Value 
*Size, unsigned AlignmentInBytes) {
+  ConstantInt *Byte;
+  switch (CGF.getLangOpts().getTrivialAutoVarInit()) {
+  case LangOptions::TrivialAutoVarInitKind::Uninitialized:
+// Nothing to initialize.
+return;
+  case LangOptions::TrivialAutoVarInitKind::Zero:
+Byte = CGF.Builder.getInt8(0);
+break;
+  case LangOptions::TrivialAutoVarInitKind::Pattern:
+Byte = CGF.Builder.getInt8(0xAA);
+break;
+  }
+  CGF.Builder.CreateMemSet(AI, Byte, Size, AlignmentInBytes);
+}
+
 /// getBuiltinLibFunction - Given a builtin id for a function like
 /// "__builtin_fabsf", return a Function* for "fabsf".
 llvm::Constant *CodeGenModule::getBuiltinLibFunction(const FunctionDecl *FD,
@@ -2259,6 +2275,7 @@
 .getQuantity();
 AllocaInst *AI = Builder.CreateAlloca(Builder.getInt8Ty(), Size);
 AI->setAlignment(SuitableAlignmentInBytes);
+initializeAlloca(*this, AI, Size, SuitableAlignmentInBytes);
 return RValue::get(AI);
   }
 
@@ -2271,6 +2288,7 @@
 CGM.getContext().toCharUnitsFromBits(AlignmentInBits).getQuantity();
 AllocaInst *AI = Builder.CreateAlloca(Builder.getInt8Ty(), Size);
 AI->setAlignment(AlignmentInBytes);
+initializeAlloca(*this, AI, Size, AlignmentInBytes);
 return RValue::get(AI);
   }
 


Index: test/CodeGenCXX/trivial-auto-var-init.cpp
===
--- test/CodeGenCXX/trivial-auto-var-init.cpp
+++ test/CodeGenCXX/trivial-auto-var-init.cpp
@@ -173,6 +173,34 @@
   used(ptr);
 }
 
+// UNINIT-LABEL:  test_alloca(
+// ZERO-LABEL:test_alloca(
+// ZERO:  %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// ZERO-NEXT: %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align [[ALIGN:[0-9]+]]
+// ZERO-NEXT: call void @llvm.memset{{.*}}(i8* align [[ALIGN]] %[[ALLOCA]], i8 0, i64 %[[SIZE]], i1 false)
+// PATTERN-LABEL: test_alloca(
+// PATTERN:   %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// PATTERN-NEXT:  %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align [[ALIGN:[0-9]+]]
+// PATTERN-NEXT:  call void @llvm.memset{{.*}}(i8* align [[ALIGN]] %[[ALLOCA]], i8 -86, i64 %[[SIZE]], i1 false)
+void test_alloca(int size) {
+  void *ptr = __builtin_alloca(size);
+  used(ptr);
+}
+
+// UNINIT-LABEL: