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

2019-04-11 Thread 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

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

2019-04-11 Thread 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

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

2019-04-11 Thread 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

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

2019-04-11 Thread 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

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

2019-04-11 Thread 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 >

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

2019-04-11 Thread 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

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

2019-04-11 Thread 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

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

2019-04-11 Thread 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

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

2019-04-11 Thread 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:

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

2019-04-11 Thread 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,

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

2019-04-11 Thread 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

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

2019-04-11 Thread 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

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

2019-04-11 Thread 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

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

2019-04-10 Thread 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/

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

2019-04-10 Thread 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