vzakhari marked an inline comment as done.
vzakhari added a comment.

In D63846#1574102 <https://reviews.llvm.org/D63846#1574102>, @rjmccall wrote:

> Please don't check IR names in test output.  That actually includes anonymous 
> names like `%2`; these should always be tested with FileCheck variables.  I 
> suggest using `%.*` as the pattern; if you're matching the LHS of an LLVM 
> assignment, that shouldn't have problems with accidentally matching too much.


I agree that checking IR names is a bad practice.  I can change all these tests 
so that they either use FileCheck variables or -fno-discard-value-names (e.g. 
for clang/test/CodeGenOpenCLCXX/addrspace-of-this.cl, which relies on the names 
for checking) - would that be an appropriate solution?



================
Comment at: clang/test/CodeGenOpenCL/address-spaces-conversions.cl:16
   arg_gen = &var_priv; // implicit cast with obtaining adr, private -> generic
-  // CHECK: %{{[0-9]+}} = addrspacecast i32* %var_priv to i32 addrspace(4)*
+  // CHECK: %var_priv.ascast = addrspacecast i32* %var_priv to i32 
addrspace(4)*
   // CHECK-NOFAKE-NOT: addrspacecast
----------------
erichkeane wrote:
> You probably don't want to remove the wildcard here.  If this is built 
> without names being saved, this change will fail.
In the tests that already use variable names I stick to checking the names, 
e.g. this test already assumes that the names are preserved (%var_priv in this 
check), so I am just checking that 'ascast' suffix is added.  I guess I will 
have to add -fno-discard-value-names for this test - do you agree?


Repository:
  rC Clang

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

https://reviews.llvm.org/D63846



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

Reply via email to