efriedma added a comment.

I noted the cases where it looks like the undef->poison change might actually 
impact code using compiler intrinsic functions that have external 
specifications.  The relevant specifications say the elements in question are 
"undefined", without really specifying what that means.

Currently, for the Intel intrinsics, we treat "undefined" as something more 
conservative than LLVM undef; see 
https://github.com/llvm/llvm-project/blob/d2012d965d60c3258b3a69d024491698f8aec386/clang/lib/CodeGen/CGBuiltin.cpp#L12483
 .  Maybe we should make the cast intrinsics more conservative to match.  And 
maybe we should do the same for OpenCL.  Would need to do some backend work to 
make sure we don't regress the generated code, though.

For __builtin_shufflevector, I think I'm fine with this changing the "-1" to 
mean poison; we don't have any external spec to conform to, and anyone 
explicitly passing -1 should know what they're doing.  But maybe worth noting 
in the clang documentation.



================
Comment at: clang/test/CodeGen/X86/avx-builtins.c:182
   // CHECK-LABEL: test_mm256_castsi128_si256
-  // CHECK: shufflevector <2 x i64> %{{.*}}, <2 x i64> %{{.*}}, <4 x i32> <i32 
0, i32 1, i32 undef, i32 undef>
+  // CHECK: shufflevector <2 x i64> %{{.*}}, <2 x i64> %{{.*}}, <4 x i32> <i32 
0, i32 1, i32 poison, i32 poison>
   return _mm256_castsi128_si256(A);
----------------
This change might be visible to user code.


================
Comment at: clang/test/CodeGen/builtinshufflevector2.c:41
 // CHECK: store <4 x float> [[V]], <4 x float>* {{%.*}}
   *A = __builtin_shufflevector( x, y, 0, 4, -1, 5 );
 }
----------------
This might be visible to user code.


================
Comment at: clang/test/CodeGenOpenCL/preserve_vec3.cl:21
   // CHECK: %[[LOAD_A:.*]] = load <3 x float>, <3 x float> addrspace(1)* %a, 
align 16
-  // CHECK: %[[ASTYPE:.*]] = shufflevector <3 x float> %[[LOAD_A]], <3 x 
float> poison, <4 x i32> <i32 0, i32 1, i32 2, i32 undef>
+  // CHECK: %[[ASTYPE:.*]] = shufflevector <3 x float> %[[LOAD_A]], <3 x 
float> poison, <4 x i32> <i32 0, i32 1, i32 2, i32 poison>
   // CHECK: store <4 x float> %[[ASTYPE:.*]], <4 x float> addrspace(1)* %b, 
align 16
----------------
This change might be visible to user code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103874

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

Reply via email to