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. > 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
