On Mon, Feb 20, 2023 at 5:23 PM Tobias Burnus <tob...@codesourcery.com> wrote: > > Hi Richard, hi all, > > On 20.02.23 13:46, Richard Biener wrote: > > + /* TODO: A more middle-end friendly alternative would be to use > > NULL_TREE > > + as upper bound and store the value, e.g. as GFC_DECL_STRING_LEN. > > + Caveat: this requires some cleanup throughout the code to > > consistently > > + use some wrapper function. */ > > + gcc_assert (TREE_CODE (TYPE_SIZE_UNIT (type)) == SAVE_EXPR); > > + tree tmp = TREE_TYPE (TYPE_SIZE (eltype)); > > > > ... > > > > you are probably breaking type sharing here. You could use > > build_array_type_1 and pass false for 'shared' to get around that. Note > > that there's also canonical type building done in case 'eltype' is not > > canonical itself. > > My feeling is that this is already somewhat broken. Currently, there > is one type per decl as each has its own artificial length variable. > I have no idea how this will be handled in the ME in terms of alias > analysis. And whether shared=false makes sense here and what effect > is has. (Probably yes.) > > In principle, > integer(kind=8) .str., .str2; > character(kind=1)[1:.str] * str; > character(kind=1)[1:.str2] * str2; > have the same type and iff .str == .str at runtime, they can alias. > Example: > str2 = str; > .str2 = .str; > > I have no idea how the type analysis currently works (with or without > SAVE_EXPR) > nor what effect shared=false has in this case.
alias analysis for array types looks only at the element type > > The solution to the actual problem is a hack - you are relying on > > re-evaluation of TYPE_SIZE, and for that, only from within accesses > > from inside the frontend? > > I think this mostly helps with access inside the FE of the type 'size = > TYPE_SIZE_UNIT(type)', which is used surprisingly often and is often > directly evaluated (i.e. assigned to a temporary). that's what I thought > > Since gimplification will produce the result into a single temporary again, > > re-storing the "breakage". > > So, does it_really_ fix things? > > It does seem to fix cases which do 'size = TYPE_SIZE_UNIT (type);' in > the front end and then uses this size expression. Thus, there are fixed. > However, there are many cases where things go wrong - with and without > the patch. I keep discovering more and more :-( I guess test coverage isn't too great with this feature then ;) > * * * > > I still think that the proper way is to have NULL_TREE as upper value > would be better in several ways, except that there is (too) much code Yep. > which relies on TYPE_UNIT_SIZE to work. (There are 117 occurrences). > Additionally, there is more code doing assumptions in this area. > > Thus, the question is whether it makes sense as hackish partial solution > or whether it should remain in the current broken stage until it is > fixed properly. I wonder if it makes more sense to individually fix the places using TYPE_UNIT_SIZE in a wrong way? You'd also get only "partial" fixes, but at least those will be true and good? Otherwise I defer to frontend maintainers if they agree to put in a (partially working) hack like this. Richard. > Tobias, > > who would like to have more time for fixing such issues. > > ----------------- > Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 > München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas > Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht > München, HRB 106955
