erichkeane added inline comments.

================
Comment at: clang/test/C/C2x/n2900_n3011_2.c:51
+  // CHECK: define {{.*}} void @test_vla
+  // CHECK-NEXT: entry:
+  // CHECK-NEXT: %[[I:.+]] = alloca i32
----------------
so 'entry' isn't really a stable name AFAIK.  So this might fail in test 
configs that do the 'erase names' thing.  That said, a buildbot hasn't caught 
one of those in a long time, so *shrug*.


================
Comment at: clang/test/C/C2x/n2900_n3011_2.c:55
+  // CHECK-NEXT: store i32 12, ptr %[[I]]
+  // CHECK-NEXT: %[[MEM0:.+]] = load i32, ptr %[[I]]
+  // CHECK-NEXT: %[[MEM1:.+]] = zext i32 %[[MEM0]] to i64
----------------
aaron.ballman wrote:
> erichkeane wrote:
> > These MEM# names are horrible to read :/  But the test is doing the right 
> > thing it appears.
> If you have suggestions for better names, I'm happy to use them.
`I`: `I_PTR`
`MEM0`: `I_VAL`
`MEM1`: `NUM_ELTS` (or `I_AS_64B`?).
`MEM3`: `COPY_BYTES`

Though, this all becomes a bit easier with 'i' in code being named `num_elts` 
or something.
`NUM_ELTS_PTR`
`NUM_ELTS`
`NUM_ELTS_EXT`
`BYTES_TO_COPY`




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

https://reviews.llvm.org/D147349

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

Reply via email to