On Tue, May 29, 2012 at 11:17 AM, Eric Botcazou <ebotca...@adacore.com> wrote: >> Yes, I realize that, but it looks like that code is not doing >> something because old integrate.c choked on it. Quoting that part of >> the patch: >> >> Index: expmed.c >> =================================================================== >> --- expmed.c (revision 187936) >> +++ expmed.c (working copy) >> @@ -1866,6 +1866,9 @@ extract_fixed_bit_field (enum machine_mode tmode, >> shift it so it does. */ >> /* Maybe propagate the target for the shift. */ >> /* But not if we will return it--could confuse integrate.c. */ >> + /* ??? What does the above comment mean in relation to the code >> + below? NB, integrate.c is no more, so I guess it can't be >> + confused by anything anymore... */ >> rtx subtarget = (target != 0 && REG_P (target) ? target : 0); >> if (tmode != mode) subtarget = 0; >> op0 = expand_shift (RSHIFT_EXPR, mode, op0, bitpos, subtarget, 1); >> >> Does that "But not if..." comment refer to the "if (tmode != mode) >> subtarget = 0;" line? Of so, can/should we drop that line (as well as >> the comment)? I don't know this part of the compiler well enough to >> tell. > > I don't think it's a direct reference. There is the same pattern at the end > of > the function, where TARGET has already been zeroed if mode != tmode. If the > mode don't match, expand_shift cannot reuse [SUB]TARGET as-is, so the code > consistently avoids to pass it. > > I'd just drop the dangling reference to integrate.c and reformat the line > > if (tmode != mode) subtarget = 0; > > into > > if (mode != mode) > subtarget = 0; > > to make it more closely ressemble the other instance.
Done, updated diff attached. OK for trunk? Ciao! Steven
disintegrate_integrate.diff
Description: Binary data
