pcc added inline comments.
================
Comment at: include/clang/Driver/ToolChain.h:355
+ virtual LangOptions::TrivialAutoVarInitKind
+ GetDefaultTrivialAutoVarInit() const {
+ return LangOptions::TrivialAutoVarInitKind::Uninitialized;
----------------
I wouldn't introduce this function because nothing is overriding it (for now,
at least).
================
Comment at: lib/CodeGen/CGDecl.cpp:968
+ // The following value is a guaranteed unmappable pointer value and has a
+ // reperated byte-pattern which makes it easier to synthesize. We use it for
+ // pointers as well as integers so that aggregates are likely to be
----------------
*repeated
================
Comment at: lib/CodeGen/CGDecl.cpp:977
+ constexpr uint32_t SmallValue = 0x000000AA;
+ if (Ty->isIntOrIntVectorTy()) {
+ unsigned BitWidth = llvm::cast<llvm::IntegerType>(
----------------
Do you have test coverage for vectors?
================
Comment at: lib/CodeGen/CGDecl.cpp:978
+ if (Ty->isIntOrIntVectorTy()) {
+ unsigned BitWidth = llvm::cast<llvm::IntegerType>(
+ Ty->isVectorTy() ? Ty->getVectorElementType() : Ty)
----------------
Is it necessary to qualify `cast` with `llvm::`? It doesn't seem to be
qualified elsewhere in this file. (Same below.)
================
Comment at: lib/CodeGen/CGDecl.cpp:1145
+ if (Ty->isStructTy() || Ty->isArrayTy() || Ty->isVectorTy()) {
+ for (unsigned Op = 0, NumOp = constant->getNumOperands(); Op != NumOp;
+ ++Op) {
----------------
Maybe use `User::operands()` to enumerate operands here?
================
Comment at: lib/CodeGen/CGDecl.cpp:1631
+ auto initializeWhatIsTechnicallyUninitialized = [&]() {
+ if (trivialAutoVarInit !=
+ LangOptions::TrivialAutoVarInitKind::Uninitialized) {
----------------
Is it possible to rewrite this function using early returns to avoid all the
indentation below for the VLA case? I'd also put the code for non-VLAs first to
make this function easier to understand (on my first reading I somehow got the
impression that this function is *only* handling VLAs). I'm thinking maybe
something like:
```
if (trivialAutoVarInit == LangOptions::TrivialAutoVarInitKind::Uninitialized)
return;
if (!getContext().getTypeSizeInChars(type).isZero()) {
// handle non-VLAs
}
auto *VlaType =
dyn_cast_or_null<VariableArrayType>(getContext().getAsArrayType(type));
if (!VlaType)
return;
// handle VLAs
```
================
Comment at: lib/CodeGen/CGDecl.cpp:1673
+ llvm::BasicBlock *ContBB = createBasicBlock("vla-init.cont");
+ EmitBlock(LoopBB);
+ llvm::PHINode *Cur =
----------------
I think we need to check that the size is non-zero here. Otherwise we're going
to clobber whatever comes after the VLA if its size is zero (I know that C
forbids zero-sized VLAs, but I believe we support them as a GNU extension).
================
Comment at: test/CodeGenCXX/trivial-auto-var-init.cpp:5
+
+// PATTERN-NOT: undef
+// ZERO-NOT: undef
----------------
Since this is your only test involving structs and arrays, you probably want to
test them beyond the fact that they don't contain undef. I also believe that
this is testing for the absence of `undef` before the first label and not the
total absence of `undef` in the output.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54604/new/
https://reviews.llvm.org/D54604
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
