Joern Rennecke <joern.renne...@embecosm.com> writes: > Quoting Richard Sandiford <rdsandif...@googlemail.com>: >> I should probably have piped up earlier, but I'm really not sure about it. >> ADJUST_INSN_LENGTH as defined now is telling us a property of the target. >> The patch instead ties the new hook directly into the shorten_branches >> algorithm, which I think is a bad idea. >> >> IMO, the length attributes and ADJUST_INSN_LENGTH should be treated as >> a fused process: every value returned by a length attribute (whether min, >> default or current) should be filtered through ADJUST_INSN_LENGTH before >> being used. Whether ADJUST_INSN_LENGTH decreases or increases the original >> length doesn't matter, because only the output of ADJUST_INSN_LENGTH should >> be used. >> >> Joern's patch makes us use ADJUST_INSN_LENGTH for delay slot insns, >> which I agree is a good thing in itself. It's the all-important >> iteration parameter that feels wrong. >> >> TBH, I still don't understand what property of the target we're trying >> to describe here. I gather it has something to do with alignment. >> But I'd really like a description of that target property, independent >> of the shorten_branches implementation. > > It's about describing complex interactions of length adjustments that > derive from branch shortening and length added for (un)alignment for > scheduling purposes. Expressed naively, these can lead to cycles.
But shorten_branches should be written to avoid cycles, and I thought your patch did that by making sure that the address of each instruction only increases. > I could manage with the combination of length / lock_length attribute, > but there was a fine line between tuning for optimal scheduling and > risking cycles. > So, as Richard Biener has pointed out, that design was not optimal. > > What is really needed is a way to express priorities between instructions > when to inhibit length shrinkage from one iteration to another. > > Ports that have no issues with negative feedback in the length calculations > don't need to worry about priorities; the length locking mechanism > will be a no-op anyways, so the question when it is being applied is moot. > > In the case you have negative feedback, the default, to apply length > locking immediatly to all instructions, gives the fastest convergence, > although it will likely give a sub-optimal shortening solution. > > The length variation for the ARC are not alike: there are branches that > are subject to branch shortening in the usual way, but they might > shrink when other things shrink. When we are iterating starting at > minimal length and increasing lengths, these branches should be stopped > from shrinking, as they likly will grow back again in the next iteration. > OTOH there are potentially short instructions that are lengthened to > archive scheduling-dictated alignment or misalignment. These should > be allowed to change freely back and forth. Unless we have a rare > cycle that only depends on alignments. Hmm, but this is still talking in terms of shorten_branches, rather than the target property that we're trying to model. It sounds like the property is something along the lines of: some instructions have different encodings (or work-alikes with different encodings), and we want to make all the encodings available to the optimisers. Is that right? If so, that sounds like a useful thing to model, but I think it should be modelled directly, rather than as a modification to the shorten_branches algorithm. Is the alignment/misalignment explicitly modelled in the rtl? If so, how? Using labels, or something else? Richard
