On July 24, 2017 3:42:30 PM GMT+02:00, Segher Boessenkool <seg...@kernel.crashing.org> wrote: >On Mon, Jul 24, 2017 at 01:52:49PM +0100, Richard Sandiford wrote: >> Segher Boessenkool <seg...@kernel.crashing.org> writes: >> > On Mon, Jul 24, 2017 at 10:28:06AM +0100, Richard Sandiford wrote: >> >> > From what I can tell so far it makes things much harder to read. >> >> > Perhaps that is just because this is all new. >> >> >> >> Which parts specifically? E.g. is it mostly the is_a <T> (x, &y) >changes? >> >> Or the as_a <T> (x) changes too? Do you think the FOR_EACH_* >iterators >> >> also make things harder to read? Or is >machine_mode->scalar_int_mode >> >> itself a problem? >> > >> > All the as_a <T> (x) etc. looks like cuneiform to me (not just in >your >> > patch); and I cannot read cuneiform :-) >> > >> > One day I might understand why we need all this C++ inverted >syntax, >> > needless abstraction, needless generalisation, data hiding and >everything >> > else hiding. Until then, I rant. Sorry. >> > >> > The main purpose of abstraction is to make code easier to >understand and >> > to write and change, but with C++ it usually makes it harder it >seems :-( >> >> Well, as someone who was/is on the fence about the C++ thing, I >definitely >> sympathise :-) But in this particular case it really isn't (supposed >to be) >> "hey, we're using C++ now, let's add more layers!" abstraction. > >:-) > >> The same principle would have worked in C and IMO been useful in C. >> It sounds like you don't necessarily object to SCALAR_INT_TYPE_MODE >etc. >> as asserting forms of TYPE_MODE, so if the syntax is a problem, I >think: >> >> as_a <scalar_int_mode> (x) => force_scalar_int (x) >> is_a <scalar_int> (x, &y) => is_scalar_int (x, &y) >> >> would be fine too, and shorter to write. Would that be better? > >Yeah that looks better, to me at least.
I don't like that. We've settled on one style so please adhere to that. Force_ also suggests some magic semantics that are not there. So if any then try to improve the as-is stuff in general. For example it's quite awkward in switches. Richard. >> Like I say, one advantage of the new wrappers is that they force mode >> assumptions to be explicit (otherwise the compiler won't build). >That >> caught several real bugs before they had been found and fixed >upstream. >> But that argument might be too weak to support this on its own. > >It also helps compile time you said. I like that, too :-) > >> The other -- original, and IMO more compelling -- motivation is to >make >> it easier to add variable-sized modes without having to worry about >the >> possibility that scalar or complex modes could be variable-sized >> (because that would be much more invasive). We could instead have >kept >> everything as machine_mode, made GET_MODE_SIZE always be variable, >and >> forced the result to a constant whenever it's "known" to be constant >> for some reason. The difficulty with that is that it would be very >hard >> to get right if not testing SVE, and even with SVE you would need >just >> the right input code to trigger the problem. So what we wanted to do >> was make it "easy" for people to know when variable-sized modes were >a >> concern even if they weren't thinking about or testing SVE. This >> scalar/not scalar or complex/not complex choices aren't architecture- >> specific in the same way. >> >> With this approach we only needed to force a variable size or offset >to >> a constant in a few places, and those cases were easy to justify from >> context. > >Yes, it is clear some changes are needed -- I just hope the changes >can make the code easier to understand, instead of more complex. > > >Segher