On 7/30/20 10:57 AM, Jakub Jelinek wrote:
On Thu, Jul 30, 2020 at 10:16:46AM -0400, Jason Merrill wrote:
It checks the various std::bit_cast requirements, when not constexpr
evaluated acts pretty much like VIEW_CONVERT_EXPR of the source argument
to the destination type and the hardest part is obviously the constexpr
evaluation. I couldn't use the middle-end native_encode_initializer,
because it needs to handle the C++ CONSTRUCTOR_NO_CLEARING vs. negation of
that
CONSTRUCTOR_NO_CLEARING isn't specific to C++.
That is true, but the middle-end doesn't really use that much (except one
spot in the gimplifier and when deciding if two constructors are equal).
value initialization of missing members if there are any etc.
Doesn't native_encode_initializer handle omitted initializers already? Any
elements for which the usual zero-initialization is wrong should already
have explicit initializers by the time we get here.
It assumes zero initialization for everything that is not explicitly
initialized.
When it is used in the middle-end, CONSTRUCTORs are either used in
initializers of non-automatic vars and then they are zero initialized, or
for automatic variables only empty CONSTRUCTORs are allowed.
So, what it does is memset the memory to zero before trying to fill in the
individual initializers for RECORD_TYPE/ARRAY_TYPE.
Even if we are guaranteed that (what guarantees it?) when __builtin_bit_cast
is constexpr evaluated no initializer will be omitted if may be value
initialized
to something other than all zeros,
This is established by digest_init/process_init_constructor_record,
which replace implicit value-initialization with explicit values when
it's not simple zero-initialization.
we still need to track what bits are well
defined and what are not (e.g. any padding in there).
Thinking about it now, maybe the mask handling for !CONSTRUCTOR_NO_CLEARING
is incorrect though if there are missing initializers, because those omitted
initializers still could have paddings with unspecified content, right?
Zero-initialization (and thus trivial value-initialization) of a class
also zeroes out padding bits, so when !CONSTRUCTOR_NO_CLEARING all bits
should be well defined.
needs to handle bitfields even if they don't have an integral representative
Can we use TYPE_MODE for this?
When the bitfield representative is an array, it will have BLKmode
TYPE_MODE, and we need to find some suitable other integral type.
The bit-field handling for BLKmode representatives can be copied to the
middle-end
implementation.
These all sound like things that could be improved in
native_encode_initializer rather than duplicate a complex function.
I think in the end only small parts of it are actually duplicated.
It is true that both native_encode_initializer and the new C++ FE
counterpart are roughly 310 lines long, but the mask handling the latter
performs is unlikely useful for the middle-end (and if added there it would
need to allow it thus to be NULL and not filled), on the other side the very
complex handling the middle-end version needs to do where it can be asked
only for a small portion of the memory complicates things a lot.
Maybe we could say that non-NULL mask is only supported for the case where
one asks for everything and have some assertions for that, but I fear we'd
still end up with ~ 450-500 lines of code in the middle-end compared to the
current 310.
I guess I can try to merge the two back in one with optional mask and see
how does it look like. But most likely the !CONSTRUCTOR_NO_CLEARING
handling etc. would need to be conditionalized on this special C++ mode
(mask non-NULL) etc.
+ if (REFERENCE_REF_P (arg))
+ arg = TREE_OPERAND (arg, 0);
Why?
I've added it so that the decay_conversion later on didn't mishandle
references to arrays.
+ if (error_operand_p (arg))
+ return error_mark_node;
+
+ if (!type_dependent_expression_p (arg))
+ {
+ if (TREE_CODE (TREE_TYPE (arg)) == ARRAY_TYPE)
+ {
+ /* Don't perform array-to-pointer conversion. */
+ arg = mark_rvalue_use (arg, loc, true);
+ if (!complete_type_or_maybe_complain (TREE_TYPE (arg), arg, complain))
+ return error_mark_node;
+ }
But then I've added the above to match more the __builtin_bit_cast behavior
in clang, so maybe the if (REFERENCE_REF_P (arg)) is not needed anymore.
For the std::bit_cast uses, the argument will be always a reference, never
an array directly and not sure if we want to encourage people to use the
builtin directly in their code.
+ else
+ arg = decay_conversion (arg, complain);
+
+ if (error_operand_p (arg))
+ return error_mark_node;
+
+ arg = convert_from_reference (arg);
+ if (!trivially_copyable_p (TREE_TYPE (arg)))
+ {
+ error_at (cp_expr_loc_or_loc (arg, loc),
+ "%<__builtin_bit_cast%> source type %qT "
+ "is not trivially copyable", TREE_TYPE (arg));
+ return error_mark_node;
+ }
+ if (!dependent_type_p (type)
+ && !cp_tree_equal (TYPE_SIZE_UNIT (type),
+ TYPE_SIZE_UNIT (TREE_TYPE (arg))))
+ {
+ error_at (loc, "%<__builtin_bit_cast%> source size %qE "
+ "not equal to destination type size %qE",
+ TYPE_SIZE_UNIT (TREE_TYPE (arg)),
+ TYPE_SIZE_UNIT (type));
+ return error_mark_node;
+ }
+ }
+
+ tree ret = build_min (BIT_CAST_EXPR, type, arg);
+ SET_EXPR_LOCATION (ret, loc);
+ return ret;
+}
+
#include "gt-cp-semantics.h"
--- gcc/cp/tree.c.jj 2020-07-18 10:48:51.738489259 +0200
+++ gcc/cp/tree.c 2020-07-18 10:52:46.188021206 +0200
@@ -3908,6 +3908,11 @@ cp_tree_equal (tree t1, tree t2)
return false;
return true;
+ case BIT_CAST_EXPR:
+ if (!same_type_p (TREE_TYPE (t1), TREE_TYPE (t2)))
+ return false;
+ break;
Add this to the *_CAST_EXPR cases like you do in cp_walk_subtrees?
Ok.
+ {
+ if (type == orig_type)
+ error_at (loc, "%qs is not constant expression because %qT is "
+ "a union type", "__builtin_bit_cast", type);
"not a constant expression"; you're missing the "a" in all of these.
Thanks, will change.
Jakub
