https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83801

--- Comment #7 from Georg-Johann Lay <gjl at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #5)
> I think the testcases just make invalid assumptions.

Huh?  Which assumptions specifically?

1) There are objects of static storage duration in some non-generic AS.

2) Object locations must match accesses (which is not the case for the
   code v8 generates for the test cases): Flash reads for flash, RAM reads
   for RAM.  Any mixing is wrong code.

3) If the compiler puts some object that should go into some AS into a
   different AS, then it must prove that all accesses are also to that
   other AS (including accesses via addresses that may escape and acceses
   via inline asm).

4) For avr, the whole point of ASes is to save precious RAM and put stuff
   into flash.  As accessing flash needs different instructions, different
   addressing modes and registers, ASes are used (or one must resort to
   inline asm).  Hence dropping AS and using generic AS makes only sense
   from the perspective of optimization if the original AS object is no
   more needed (otherwise the entity will be there at least twice, once
   in flash and once in RAM).

> In any case, the change that matters here is likely my r254930 or r255285,
> and one could do something like add say:
>   && (ADDR_SPACE_GENERIC_P (TYPE_ADDR_SPACE (TREE_TYPE (decl)))
>       || !AGGREGATE_TYPE_P (TREE_TYPE (decl)))
> to decl_constant_value_1 conditions to prevent it from looking at aggregate
> initializers in non-generic address spaces.
> Perhaps there should be also some cap on the size of decl that is optimized
> regardless of the address space, e.g. 7.x and earlier had:
>       || TREE_CODE (TREE_TYPE (exp)) == ARRAY_TYPE
>       || DECL_MODE (exp) == BLKmode)
> condition to punt.  Now, sometimes it is beneficial to have even BLKmode or
> array decls to go through, say if we have str[5], similarly
> const_var.field,then not punting on those will allow it to be optimized into
> constant, while 7.x couldn't.

As mentioned in 4) above, "optimizing" to CST makes only sense if the original
decl can be dropped; at least for avr.  The point is not speed of access but
saving RAM memory and using flash instead.  Keep in mind that .rodata must be
in RAM; therefore __flash and other ASes need specific sections to locate in
flash, namely .progmem* sections.

At least it should be decided whether STRING_CST is legitimate for ASes (and
hence the target hooks should handle them, dito for other CSTs) or if it is
incorrect to attach ASes to them (and decls should be used instead like v7
did).

> But, with something large and non-constant
> access it might actually regress (duplicate the constant).

Duplicating is really bad.  The whole point of ASes for avr is so save memory,
RAM in particular.

Actually, anything in flash must be const (at least for avr) and hence has a
known layout, size and initializer.  Due to this /any/ char[] in AS could be
represented as STRING_CST.  If the FE doesn't know, maybe a new target hook is
needed?

Common idioms to put strings in flash are:

const __flash char string[] = "flash-string";

both outside of functions and as local static inside of functions, and

#define FSTR(X) ((const __flash char[]) { X } )

const __flash char* const __flash animals[] =
{
  FSTR ("mite"), FSTR ("slug"), FSTR ("sloth")
};

(which is rejected for local static due to "&& current_function_decl" in
c-parser.c).

Reply via email to