| Message ID | 20161018122421.64118-1-Vincent.Riera@imgtec.com |
|---|---|
| State | Changes Requested |
| Headers | show |
Hello, On Tue, 18 Oct 2016 13:24:21 +0100, Vicente Olivert Riera wrote: > MIPS Creator CI20 has an Ingenic JZ4780 SoC which features an XBurst > CPU. The FPU on that CPU has a bug that can generate incorrect results > in certain cases. The problem shows up when you have several of these > instructions in sequence with dependant operands. > > Using the -mno-fused-madd option prevents gcc from emitting these > instructions. > > More details here: > - https://groups.google.com/forum/#!topic/mips-creator-ci20/spDB2jjbizM > - https://android.googlesource.com/platform/build/+/90ce45347064210585a3a1f59a0514c22c753c8a > > Signed-off-by: Vicente Olivert Riera <Vincent.Riera@imgtec.com> > --- > configs/ci20_defconfig | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/configs/ci20_defconfig b/configs/ci20_defconfig > index 0455170..1894554 100644 > --- a/configs/ci20_defconfig > +++ b/configs/ci20_defconfig > @@ -8,6 +8,8 @@ BR2_PACKAGE_HOST_LINUX_HEADERS_CUSTOM_3_18=y > > # system > BR2_TARGET_GENERIC_GETTY_PORT="ttyS4" > +# Avoid FPU bug > +BR2_TARGET_OPTIMIZATION="-mno-fused-madd" Unfortunately, this means that other people using the same MIPS CPU, but not using your defconfig will fall into this issue. For the Intel X1000, which also had a bug, we've added a separate entry in arch/Config.in.x86. Should we be doing the same here for this MIPS CPU ? Note: this is *really* a question. I am not sure we want to make such a change to the MIPS CPU selection. Thomas
Hello Thomas, On 18/10/16 13:54, Thomas Petazzoni wrote: > Hello, > > On Tue, 18 Oct 2016 13:24:21 +0100, Vicente Olivert Riera wrote: >> MIPS Creator CI20 has an Ingenic JZ4780 SoC which features an XBurst >> CPU. The FPU on that CPU has a bug that can generate incorrect results >> in certain cases. The problem shows up when you have several of these >> instructions in sequence with dependant operands. >> >> Using the -mno-fused-madd option prevents gcc from emitting these >> instructions. >> >> More details here: >> - https://groups.google.com/forum/#!topic/mips-creator-ci20/spDB2jjbizM >> - https://android.googlesource.com/platform/build/+/90ce45347064210585a3a1f59a0514c22c753c8a >> >> Signed-off-by: Vicente Olivert Riera <Vincent.Riera@imgtec.com> >> --- >> configs/ci20_defconfig | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/configs/ci20_defconfig b/configs/ci20_defconfig >> index 0455170..1894554 100644 >> --- a/configs/ci20_defconfig >> +++ b/configs/ci20_defconfig >> @@ -8,6 +8,8 @@ BR2_PACKAGE_HOST_LINUX_HEADERS_CUSTOM_3_18=y >> >> # system >> BR2_TARGET_GENERIC_GETTY_PORT="ttyS4" >> +# Avoid FPU bug >> +BR2_TARGET_OPTIMIZATION="-mno-fused-madd" > > Unfortunately, this means that other people using the same MIPS CPU, > but not using your defconfig will fall into this issue. True. > For the Intel X1000, which also had a bug, we've added a separate > entry in arch/Config.in.x86. Should we be doing the same here for this > MIPS CPU ? > > Note: this is *really* a question. I am not sure we want to make such a > change to the MIPS CPU selection. We could add an Ingenic XBurst entry in the MIPS CPU selection, and then add that -mno-fused-madd option to the wrapper based on that selection. If you are OK with that, please let me know and I'll cook the patch. Vincent. > Thomas >
>>>>> "Vicente" == Vicente Olivert Riera <Vincent.Riera@imgtec.com> writes: Hi, >> Note: this is *really* a question. I am not sure we want to make such a >> change to the MIPS CPU selection. > We could add an Ingenic XBurst entry in the MIPS CPU selection, and then > add that -mno-fused-madd option to the wrapper based on that selection. > If you are OK with that, please let me know and I'll cook the patch. Yes, I believe this is the way we want to move forward.
Hello, On Wed, 19 Oct 2016 09:47:03 +0100, Vicente Olivert Riera wrote: > We could add an Ingenic XBurst entry in the MIPS CPU selection, and then > add that -mno-fused-madd option to the wrapper based on that selection. > > If you are OK with that, please let me know and I'll cook the patch. If I understand correctly: - JZ4780 is the SoC - XBurst is the core - MIPS32r2 is the ISA So indeed, it makes sense to add an option for XBurst, in order to mimic what we do on ARM. Ideally, we should mimic what we do on ARM, and only list in "Target architecture variants" the cores and not the ISA. How many vendors are doing MIPS cores, and how many cores are they doing? If you look at ARM, we have the following situation: - Freescale, TI, Atmel, Marvell, Qualcomm, Nvidia, etc. are doing SoCs They are way too many for Buildroot to have a list of them. - Very few vendors are doing cores. Most of the SoC vendors are using the cores from ARM. For example, we have ARM926, Cortex-A7, Cortex-A8, Cortex-A53, etc. Those are the ones that are listed in "Target Architecture Variant", and we use the selected option to know the ISA. - ISAs are ARMv4, ARMv5, ARMv6, ARMv7, ARMv8, with a few variants. So having a similar model for MIPS would be ideal. Best regards, Thomas
Hello Peter and Thomas, thanks both of you for your comments. I have marked this patch as changes requested and I've already sent a 2-patch series to add support for XBurst cores and select that kind of core in the ci20_defconfig. Regards, Vincent On 19/10/16 10:32, Thomas Petazzoni wrote: > Hello, > > On Wed, 19 Oct 2016 09:47:03 +0100, Vicente Olivert Riera wrote: > >> We could add an Ingenic XBurst entry in the MIPS CPU selection, and then >> add that -mno-fused-madd option to the wrapper based on that selection. >> >> If you are OK with that, please let me know and I'll cook the patch. > > If I understand correctly: > > - JZ4780 is the SoC > - XBurst is the core > - MIPS32r2 is the ISA > > So indeed, it makes sense to add an option for XBurst, in order to > mimic what we do on ARM. > > Ideally, we should mimic what we do on ARM, and only list in > "Target architecture variants" the cores and not the ISA. How many > vendors are doing MIPS cores, and how many cores are they doing? > > If you look at ARM, we have the following situation: > > - Freescale, TI, Atmel, Marvell, Qualcomm, Nvidia, etc. are doing SoCs > They are way too many for Buildroot to have a list of them. > > - Very few vendors are doing cores. Most of the SoC vendors are using > the cores from ARM. For example, we have ARM926, Cortex-A7, Cortex-A8, > Cortex-A53, etc. > > Those are the ones that are listed in "Target Architecture Variant", > and we use the selected option to know the ISA. > > - ISAs are ARMv4, ARMv5, ARMv6, ARMv7, ARMv8, with a few variants. > > So having a similar model for MIPS would be ideal. > > Best regards, > > Thomas >
On 19-10-16 11:32, Thomas Petazzoni wrote: > Hello, > > On Wed, 19 Oct 2016 09:47:03 +0100, Vicente Olivert Riera wrote: > >> We could add an Ingenic XBurst entry in the MIPS CPU selection, and then >> add that -mno-fused-madd option to the wrapper based on that selection. >> >> If you are OK with that, please let me know and I'll cook the patch. > > If I understand correctly: > > - JZ4780 is the SoC > - XBurst is the core > - MIPS32r2 is the ISA > > So indeed, it makes sense to add an option for XBurst, in order to > mimic what we do on ARM. > > Ideally, we should mimic what we do on ARM, and only list in > "Target architecture variants" the cores and not the ISA. How many > vendors are doing MIPS cores, and how many cores are they doing? I believe that for MIPS, every vendor uses their own core, and that there is an almost one-to-one mapping between SoC family and core. AFAIU it's only the instruction set that is standardized. My gcc supports 74 cores... Do we really want to add all of these? That said, now we already have a bit a mixed situation: we have a few processors and a few generic options in Config.in.mips. But I expect that Buildroot will often be used on SoCs where either Buildroot or GCC doesn't have a processor-specific option yet, so the generic ones will still be needed. > If you look at ARM, we have the following situation: > > - Freescale, TI, Atmel, Marvell, Qualcomm, Nvidia, etc. are doing SoCs > They are way too many for Buildroot to have a list of them. > > - Very few vendors are doing cores. Most of the SoC vendors are using > the cores from ARM. For example, we have ARM926, Cortex-A7, Cortex-A8, > Cortex-A53, etc. That's the difference of course. ARM owns the core, while MIPS is a more or less open architecture. Not open enough, of course, which is why RISC-V was started. Regards, Arnout > Those are the ones that are listed in "Target Architecture Variant", > and we use the selected option to know the ISA. > > - ISAs are ARMv4, ARMv5, ARMv6, ARMv7, ARMv8, with a few variants. > > So having a similar model for MIPS would be ideal. > > Best regards, > > Thomas >
Hello, On Fri, 21 Oct 2016 21:45:52 +0200, Arnout Vandecappelle wrote: > > Ideally, we should mimic what we do on ARM, and only list in > > "Target architecture variants" the cores and not the ISA. How many > > vendors are doing MIPS cores, and how many cores are they doing? > > I believe that for MIPS, every vendor uses their own core, and that there is an > almost one-to-one mapping between SoC family and core. AFAIU it's only the > instruction set that is standardized. My gcc supports 74 cores... Do we really > want to add all of these? Well, did you count for ARM? There are also many many cores supported in gcc. Admittedly probably not 74 cores, indeed. > That said, now we already have a bit a mixed situation: we have a few > processors and a few generic options in Config.in.mips. But I expect that > Buildroot will often be used on SoCs where either Buildroot or GCC doesn't have > a processor-specific option yet, so the generic ones will still be needed. OK, fair enough. Then maybe they should be clearly separated by a comment within the choice...endchoice block. Best regards, Thomas
diff --git a/configs/ci20_defconfig b/configs/ci20_defconfig index 0455170..1894554 100644 --- a/configs/ci20_defconfig +++ b/configs/ci20_defconfig @@ -8,6 +8,8 @@ BR2_PACKAGE_HOST_LINUX_HEADERS_CUSTOM_3_18=y # system BR2_TARGET_GENERIC_GETTY_PORT="ttyS4" +# Avoid FPU bug +BR2_TARGET_OPTIMIZATION="-mno-fused-madd" # kernel BR2_LINUX_KERNEL=y
MIPS Creator CI20 has an Ingenic JZ4780 SoC which features an XBurst CPU. The FPU on that CPU has a bug that can generate incorrect results in certain cases. The problem shows up when you have several of these instructions in sequence with dependant operands. Using the -mno-fused-madd option prevents gcc from emitting these instructions. More details here: - https://groups.google.com/forum/#!topic/mips-creator-ci20/spDB2jjbizM - https://android.googlesource.com/platform/build/+/90ce45347064210585a3a1f59a0514c22c753c8a Signed-off-by: Vicente Olivert Riera <Vincent.Riera@imgtec.com> --- configs/ci20_defconfig | 2 ++ 1 file changed, 2 insertions(+)