ManuelJBrito added inline comments.
================
Comment at: clang/include/clang/Basic/Builtins.def:658
BUILTIN(__builtin_call_with_static_chain, "v.", "nt")
+BUILTIN(__builtin_nondet, "v.", "nt")
----------------
erichkeane wrote:
> aaron.ballman wrote:
> > aaron.ballman wrote:
> > > shafik wrote:
> > > > erichkeane wrote:
> > > > > Not a fan of the name in general, it doesn't really explain what is
> > > > > happening. Perhaps `__builtin_nondeterministic_value` or something?
> > > > >
> > > > > I also wonder if others might have a better name :)
> > > > `__builtin_indeterminate_value` wdyt?
> > > I think `__builtin_unspecified_value()` makes sense, as that's what
> > > you're getting back (there is a valid value, but you may get a different
> > > value on every call). I don't think `indeterminate` makes sense because
> > > that implies it may return trap values, and I'm guessing that's not
> > > behavior we want here (or am I wrong about that)?
> > >
> > > Also, should this be usable within a constant expression? I would assume
> > > not; we should have a test case showing you can't use it in such a
> > > context.
> > Why does this require custom typechecking?
> Our issue last time with "unspecified" value was that this has a distinct
> meaning in C/C++, so we don't want to use that word.
> I don't think `indeterminate` makes sense because that implies it may return
> trap values, and I'm guessing that's not behavior we want here (or am I wrong
> about that)?
The idea is for it to return some correct value of the same type as the
argument, so no trap values.
Can we settle on `__builtin_nondeterministic_value`?
================
Comment at: clang/include/clang/Basic/Builtins.def:658
BUILTIN(__builtin_call_with_static_chain, "v.", "nt")
+BUILTIN(__builtin_nondet, "v.", "nt")
----------------
ManuelJBrito wrote:
> erichkeane wrote:
> > aaron.ballman wrote:
> > > aaron.ballman wrote:
> > > > shafik wrote:
> > > > > erichkeane wrote:
> > > > > > Not a fan of the name in general, it doesn't really explain what is
> > > > > > happening. Perhaps `__builtin_nondeterministic_value` or
> > > > > > something?
> > > > > >
> > > > > > I also wonder if others might have a better name :)
> > > > > `__builtin_indeterminate_value` wdyt?
> > > > I think `__builtin_unspecified_value()` makes sense, as that's what
> > > > you're getting back (there is a valid value, but you may get a
> > > > different value on every call). I don't think `indeterminate` makes
> > > > sense because that implies it may return trap values, and I'm guessing
> > > > that's not behavior we want here (or am I wrong about that)?
> > > >
> > > > Also, should this be usable within a constant expression? I would
> > > > assume not; we should have a test case showing you can't use it in such
> > > > a context.
> > > Why does this require custom typechecking?
> > Our issue last time with "unspecified" value was that this has a distinct
> > meaning in C/C++, so we don't want to use that word.
> > I don't think `indeterminate` makes sense because that implies it may
> > return trap values, and I'm guessing that's not behavior we want here (or
> > am I wrong about that)?
>
> The idea is for it to return some correct value of the same type as the
> argument, so no trap values.
>
> Can we settle on `__builtin_nondeterministic_value`?
> Why does this require custom typechecking?
I wasn't sure how to have the builtin take one argument of any type.
================
Comment at: clang/include/clang/Basic/Builtins.def:658
BUILTIN(__builtin_call_with_static_chain, "v.", "nt")
+BUILTIN(__builtin_nondet, "v.", "nt")
----------------
ManuelJBrito wrote:
> ManuelJBrito wrote:
> > erichkeane wrote:
> > > aaron.ballman wrote:
> > > > aaron.ballman wrote:
> > > > > shafik wrote:
> > > > > > erichkeane wrote:
> > > > > > > Not a fan of the name in general, it doesn't really explain what
> > > > > > > is happening. Perhaps `__builtin_nondeterministic_value` or
> > > > > > > something?
> > > > > > >
> > > > > > > I also wonder if others might have a better name :)
> > > > > > `__builtin_indeterminate_value` wdyt?
> > > > > I think `__builtin_unspecified_value()` makes sense, as that's what
> > > > > you're getting back (there is a valid value, but you may get a
> > > > > different value on every call). I don't think `indeterminate` makes
> > > > > sense because that implies it may return trap values, and I'm
> > > > > guessing that's not behavior we want here (or am I wrong about that)?
> > > > >
> > > > > Also, should this be usable within a constant expression? I would
> > > > > assume not; we should have a test case showing you can't use it in
> > > > > such a context.
> > > > Why does this require custom typechecking?
> > > Our issue last time with "unspecified" value was that this has a distinct
> > > meaning in C/C++, so we don't want to use that word.
> > > I don't think `indeterminate` makes sense because that implies it may
> > > return trap values, and I'm guessing that's not behavior we want here (or
> > > am I wrong about that)?
> >
> > The idea is for it to return some correct value of the same type as the
> > argument, so no trap values.
> >
> > Can we settle on `__builtin_nondeterministic_value`?
> > Why does this require custom typechecking?
>
> I wasn't sure how to have the builtin take one argument of any type.
> Also, should this be usable within a constant expression? I would assume not;
> we should have a test case showing you can't use it in such a context.
Can you shed a light on the implications of this? Or point me to some reference?
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3064
+ case Builtin::BI__builtin_nondet: {
+ Value *Result = PoisonValue::get(ConvertType(E->getArg(0)->getType()));
+ Result = Builder.CreateFreeze(Result);
----------------
erichkeane wrote:
> Is this troublesome with some of the types that have a ConvertTypeForMem?
> That is, bool and bool vector types might need a different
> type/representation?
> https://clang.llvm.org/doxygen/CodeGenTypes_8cpp_source.html#l00091
>
>
The lowering seems to be as intended. I will add the tests.
================
Comment at: clang/test/CodeGen/builtins-nondet.c:3
+
+typedef float float4 __attribute__((ext_vector_type(4)));
+
----------------
shafik wrote:
> erichkeane wrote:
> > Add some examples for 'bool' and vector-bool as well to make sure this is
> > doing what you want.
> Does this also work for structs?
> Does this also work for structs?
No, i don't think so. What would be expected IR?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D142388/new/
https://reviews.llvm.org/D142388
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
