[PATCH] D142388: [clang] Add builtin_nondeterministic_value

ManuelJBrito added a comment. In D142388#4425246 , @aaron.ballman wrote: > In D142388#4415349 , @rsmith wrote: > >> I think `__builtin_any_value` works pretty well, and emphasizes that this >> can really return

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

aaron.ballman added a comment. In D142388#4415349 , @rsmith wrote: > I think `__builtin_any_value` works pretty well, and emphasizes that this can > really return any value. I'd also be OK with `__builtin_convenient_value`, to > emphasize that the compi

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

rsmith added a comment. In D142388#4414375 , @aaron.ballman wrote: > In D142388#4412521 , @rsmith wrote: > >> I have a concern with the name of this builtin. People are going to assume >> it produces a nondeterm

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

xbolva00 added inline comments. Comment at: clang/docs/LanguageExtensions.rst:3087 + +``__builtin_nondeterministic_value`` returns a valid nondeterministic value of the same type as the provided argument. + This text really sells this new builtin as a random val

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

xbolva00 added a comment. https://reviews.llvm.org/D136737 used 'builtin_unspecified_value' which could be okay. Are we sure we want to expose this intrinsic as it can be easily misused (as mentioned, as random generator) and/or cause security issues? Maybe only allow it in system headers (ori

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

aaron.ballman added a comment. In D142388#4412521 , @rsmith wrote: > I have a concern with the name of this builtin. People are going to assume it > produces a nondeterministic value, and use it for seeding random number > generators and similar, and wi

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

rsmith added a comment. I have a concern with the name of this builtin. People are going to assume it produces a nondeterministic value, and use it for seeding random number generators and similar, and will be surprised when the value produced is actually deterministic, and, worse, might leak i

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

zixuan-wu added inline comments. Comment at: clang/test/CodeGen/builtins-nondeterministic-value.c:26 +// CHECK-LABEL: entry +// CHECK: [[A:%.*]] = alloca double, align 8 +// CHECK: store double [[X:%.*]], ptr [[A]], align 8 ManuelJBrito wrote: > zixuan-wu wrote:

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

ManuelJBrito added inline comments. Comment at: clang/test/CodeGen/builtins-nondeterministic-value.c:26 +// CHECK-LABEL: entry +// CHECK: [[A:%.*]] = alloca double, align 8 +// CHECK: store double [[X:%.*]], ptr [[A]], align 8 zixuan-wu wrote: > hi, @ManuelJBrito

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

zixuan-wu added inline comments. Comment at: clang/test/CodeGen/builtins-nondeterministic-value.c:26 +// CHECK-LABEL: entry +// CHECK: [[A:%.*]] = alloca double, align 8 +// CHECK: store double [[X:%.*]], ptr [[A]], align 8 hi, @ManuelJBrito , because double is 4

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG450a4612c39c: [Clang] Add builtin_nondeterministic_value (authored by ManuelJBrito). Changed prior to commit: https://reviews.llvm.org/D142388?vs=

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

erichkeane added a comment. Other than the newline, the test change is acceptable to me. Commit when ready :) Comment at: clang/test/CodeGen/builtins-nondeterministic-value.c:61 +} \ No newline at end of file We do need a newline here. Repository: rG LLV

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

ManuelJBrito updated this revision to Diff 494338. ManuelJBrito added a comment. Fix vector tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142388/new/ https://reviews.llvm.org/D142388 Files: clang/docs/LanguageExtensions.rst clang/docs/Re

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

erichkeane added a comment. In D142388#4099374 , @ManuelJBrito wrote: > So this is getting some build failures: > > - https://lab.llvm.org/buildbot#builders/38/builds/9446 > - https://lab.llvm.org/buildbot#builders/245/builds/4189 > - https://lab.llvm.or

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

thakis added a comment. Reverted in adf7ffd51ee34c3a72d3168f5aed8b946ba3d2cc for now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142388/new/ https://reviews.llvm.org/D142388

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

thakis added a comment. Looks like the test fails on win: http://45.33.8.238/win/74290/step_7.txt And Mac: http://45.33.8.238/macm1/54080/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

ManuelJBrito added a comment. So this is getting some build failures: - https://lab.llvm.org/buildbot#builders/38/builds/9446 - https://lab.llvm.org/buildbot#builders/245/builds/4189 - https://lab.llvm.org/buildbot#builders/65/builds/7949 - https://lab.llvm.org/buildbot#builders/188/builds/25538

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

ManuelJBrito added a comment. Thank you all for the reviews and helping me see this patch through. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142388/new/ https://reviews.llvm.org/D142388 ___ cfe-commi

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG4a1832a5c801: [Clang] Add builtin_nondeterministic_value (authored by ManuelJBrito). Changed prior to commit: https://reviews.llvm.org/D142388?vs=

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. Last set of comments, docs and release notes look fine to me. feel free to fix the last comments while committing. Comment at: clang/lib/Sema/SemaChecking.cpp:17816

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

ManuelJBrito updated this revision to Diff 494024. ManuelJBrito added a comment. - set Invalid in type checking - add builtin documentation - add release note Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142388/new/ https://reviews.llvm.org/D142

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:17816 + if (!TyArg->isBuiltinType() && !TyArg->isVectorType()) +Diag(TheCall->getArg(0)->getBeginLoc(), diag::err_builtin_invalid_arg_type) + << 1 << /*vector, integer or floating point

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

erichkeane added a comment. In D142388#4097136 , @ManuelJBrito wrote: > Can the release note and documentation update be part of this patch or should > i create a new one ? They should be a part of this patch. Repository: rG LLVM Github Monorepo C

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

ManuelJBrito added a comment. Can the release note and documentation update be part of this patch or should i create a new one ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142388/new/ https://reviews.llvm.org/D142388 __

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

ManuelJBrito updated this revision to Diff 493984. ManuelJBrito added a comment. - Restrict builtin to base types and vectors Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142388/new/ https://reviews.llvm.org/D142388 Files: clang/include/clang/B

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

xbolva00 added a comment. Please add a release note. Thanks for this new builtin, as I was working on something very very similar - the implementation looks fine to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142388/new/ https://reviews.llv

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

aaron.ballman added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3070 + +if(Ty->isStructTy()){ + Address StructAddr = ReturnValue.getValue(); erichkeane wrote: > ManuelJBrito wrote: > > erichkeane wrote: > > > This gets REALLY complicated

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

erichkeane added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3070 + +if(Ty->isStructTy()){ + Address StructAddr = ReturnValue.getValue(); ManuelJBrito wrote: > erichkeane wrote: > > This gets REALLY complicated, you can't just create a s

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

ManuelJBrito added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3070 + +if(Ty->isStructTy()){ + Address StructAddr = ReturnValue.getValue(); erichkeane wrote: > This gets REALLY complicated, you can't just create a store, this might end

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

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

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

ManuelJBrito updated this revision to Diff 493904. ManuelJBrito retitled this revision from "[clang] Add builtin_nondet" to "[clang] Add builtin_nondeterministic_value". ManuelJBrito edited the summary of this revision. ManuelJBrito added a comment. Herald added a subscriber: mgrang. - Change nam