Hi! On Mon, Jan 20, 2020 at 08:31:43PM -0500, Nicholas Krause wrote: > On 1/20/20 6:51 PM, Segher Boessenkool wrote: > >We can (and should) use other instructions than just fsel here as well > >(say, xscmpgedp followed by xxsel). This can also work for QP float, > >which also can be TFmode, just to complicate matters more. > > > >This function is a bit big, and the logic is all over the place, but > >let's try to make it better, little by little :-) > > If your goal is make it smaller
That of course is not the goal in and of itself. The function is a little big and hard to follow, it is just run-on logic. But splitting random pieces to some new functions does not help: it just dillutes the complexity, it does not reduce it. It can of course easily have the "fsel" handling code split off, or all the scalar float handling, whichever works out best. Or maybe both. A good test for "is this a good factor" is: can you think of a short name for it, that from just looking at a call to it you understand what it does, and also understand what its arguments are? *Without* looking at the declaration of the function. > I just looked at it but wasn't sure due > to this line: > enum rtx_code code = GET_CODE (op); > in rs6000_emit_cmove and how to move it out but without duplicating it. Don't worry about lines of code. We have 60k lines or whatever, just in the C files in config/rs6000. We need code that is easier to change, code that is better maintainable, code that is easier to read. Different levels of abstraction should be in separate functions. Segher
