| Message ID | mptmu86udbz.fsf@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | rs6000: Allow FPRs to change between SDmode and DDmode [PR94254] | expand |
Hi! On Mon, Mar 23, 2020 at 06:37:52PM +0000, Richard Sandiford wrote: > g:497498c878d48754318e486428e2aa30854020b9 caused lra to cycle > on some SDmode reloads for power6. As explained in more detail > in the PR comments, the problem was a conflict between two target > hooks: rs6000_secondary_memory_needed_mode required SDmode FPR > reloads to use DDmode memory (rightly, since using SDmode memory > wouldn't make progress) but rs6000_can_change_mode_class didn't > allow FPRs to change from SDmode to DDmode. Previously lra > ignored that and changed the mode anyway. > > >From what Segher says, it sounds like the "from_size < 8 || to_size < 8" > check is mostly there for SF<->64-bit subregs, and that SDmode is stored > in the way that target-independent code expects. That is what it looks like to me, yes. > This patch therefore > allows SD<->DD changes. > > I wondered about checking for SD<->64-bit changes instead, but that > seemed like an unnecessary generalisation for this stage. Yeah, good call. > Bootstrapped and regression-tested on powerpc64le-unknown-linux-gnu, > both with default settings and with --with-cpu=power6. (For power6 > I reverted the lra patch to do the before testing, since the build > hanged otherwise.) The patch fixed the regressing testcases and > introduced no other test changes. Zdenek confirms that it also > fixes powerpc-linux-gnu builds. OK to install? (Already committed). Thanks for doing the patch! Segher > gcc/ > PR target/94254 > * config/rs6000/rs6000.c (rs6000_can_change_mode_class): Allow > FPRs to change between SDmode and DDmode.
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index 07f7cf516ba..2354aceb3f7 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -12307,6 +12307,15 @@ rs6000_can_change_mode_class (machine_mode from, if (!BYTES_BIG_ENDIAN && (to == TDmode || from == TDmode)) return false; + /* Allow SD<->DD changes, since SDmode values are stored in + the low half of the DDmode, just like target-independent + code expects. We need to allow at least SD->DD since + rs6000_secondary_memory_needed_mode asks for that change + to be made for SD reloads. */ + if ((to == DDmode && from == SDmode) + || (to == SDmode && from == DDmode)) + return true; + if (from_size < 8 || to_size < 8) return false;