[PATCH] D142388: [clang] Add builtin_nondeterministic_value

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3070
+
+    if(Ty->isStructTy()){
+      Address StructAddr = ReturnValue.getValue();
----------------
This gets REALLY complicated, you can't just create a store, this might end up hitting conversion operators/etc, and is subject to triviality/etc, and also probably needs to go through a constructor.  I suspect you're going to prefer to just decide this isn't a valid builtin for structs instead of getting bogged down in that mess.


================
Comment at: clang/lib/Sema/SemaChecking.cpp:17814
+  QualType TyArg = Arg.get()->getType();
+  TheCall->setType(TyArg);
+  return false;
----------------
So I think choosing the sub-set of types here is a much better idea, allowing any of the user-defined types is going to make your codegen for this builtin a mess.  IMO, just `isBuiltinType` + `isVectorType` is probably an acceptable list for now.  


================
Comment at: clang/test/CodeGen/builtins-nondeterministic-value.c:6
+struct testStruct{
+  int x;
+  int y;
----------------
If you're going to allow this, we need tests for unions, constructors, non-trivial types, copy assignment/etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142388