Re: [PATCH v6 1/1] gcc: config: microblaze: fix cpu version check
On 10/31/23 09:41, Frager, Neal wrote: Hi Michael, The MICROBLAZE_VERSION_COMPARE was incorrectly using strcasecmp instead of strverscmp to check the mcpu version against feature options. By simply changing the define to use strverscmp, the new version 10.0 is treated correctly as a higher version than previous versions. Signed-off-by: Neal Frager Added to commit message; Fix incorrect warning with -mcpu=10.0: warning: '-mxl-multiply-high' can be used only with '-mcpu=v6.00.a' or greater --- V1->V2: - No need to create a new microblaze specific version check routine as strverscmp is the correct solution. V2->V3: - Changed mcpu define for microblaze isa testsuite examples. V3->V4: - Added ChangeLog V4->V5: - Added testsuite ChangeLog V5->V6: - Updated testsuite ChangeLog to include all files --- gcc/ChangeLog | 4 gcc/config/microblaze/microblaze.cc | 2 +- gcc/testsuite/ChangeLog | 22 +++ .../gcc.target/microblaze/isa/bshift.c| 2 +- gcc/testsuite/gcc.target/microblaze/isa/div.c | 2 +- .../gcc.target/microblaze/isa/fcmp1.c | 2 +- .../gcc.target/microblaze/isa/fcmp2.c | 2 +- .../gcc.target/microblaze/isa/fcmp3.c | 2 +- .../gcc.target/microblaze/isa/fcmp4.c | 2 +- .../gcc.target/microblaze/isa/fcvt.c | 2 +- .../gcc.target/microblaze/isa/float.c | 2 +- .../gcc.target/microblaze/isa/fsqrt.c | 2 +- .../microblaze/isa/mul-bshift-pcmp.c | 2 +- .../gcc.target/microblaze/isa/mul-bshift.c| 2 +- gcc/testsuite/gcc.target/microblaze/isa/mul.c | 2 +- .../microblaze/isa/mulh-bshift-pcmp.c | 2 +- .../gcc.target/microblaze/isa/mulh.c | 2 +- .../gcc.target/microblaze/isa/nofcmp.c| 2 +- .../gcc.target/microblaze/isa/nofloat.c | 2 +- .../gcc.target/microblaze/isa/pcmp.c | 2 +- .../gcc.target/microblaze/isa/vanilla.c | 2 +- .../gcc.target/microblaze/microblaze.exp | 2 +- 22 files changed, 46 insertions(+), 20 deletions(-) Committed. Did you commit this patch? I only see the ChangeLog files have been updated by your commit. Am I missing something? Updated. -- Michael Eager
Re: [PATCH v6 1/1] gcc: config: microblaze: fix cpu version check
On 10/31/23 09:41, Frager, Neal wrote: Hi Michael, The MICROBLAZE_VERSION_COMPARE was incorrectly using strcasecmp instead of strverscmp to check the mcpu version against feature options. By simply changing the define to use strverscmp, the new version 10.0 is treated correctly as a higher version than previous versions. Signed-off-by: Neal Frager Added to commit message; Fix incorrect warning with -mcpu=10.0: warning: '-mxl-multiply-high' can be used only with '-mcpu=v6.00.a' or greater --- V1->V2: - No need to create a new microblaze specific version check routine as strverscmp is the correct solution. V2->V3: - Changed mcpu define for microblaze isa testsuite examples. V3->V4: - Added ChangeLog V4->V5: - Added testsuite ChangeLog V5->V6: - Updated testsuite ChangeLog to include all files --- gcc/ChangeLog | 4 gcc/config/microblaze/microblaze.cc | 2 +- gcc/testsuite/ChangeLog | 22 +++ .../gcc.target/microblaze/isa/bshift.c| 2 +- gcc/testsuite/gcc.target/microblaze/isa/div.c | 2 +- .../gcc.target/microblaze/isa/fcmp1.c | 2 +- .../gcc.target/microblaze/isa/fcmp2.c | 2 +- .../gcc.target/microblaze/isa/fcmp3.c | 2 +- .../gcc.target/microblaze/isa/fcmp4.c | 2 +- .../gcc.target/microblaze/isa/fcvt.c | 2 +- .../gcc.target/microblaze/isa/float.c | 2 +- .../gcc.target/microblaze/isa/fsqrt.c | 2 +- .../microblaze/isa/mul-bshift-pcmp.c | 2 +- .../gcc.target/microblaze/isa/mul-bshift.c| 2 +- gcc/testsuite/gcc.target/microblaze/isa/mul.c | 2 +- .../microblaze/isa/mulh-bshift-pcmp.c | 2 +- .../gcc.target/microblaze/isa/mulh.c | 2 +- .../gcc.target/microblaze/isa/nofcmp.c| 2 +- .../gcc.target/microblaze/isa/nofloat.c | 2 +- .../gcc.target/microblaze/isa/pcmp.c | 2 +- .../gcc.target/microblaze/isa/vanilla.c | 2 +- .../gcc.target/microblaze/microblaze.exp | 2 +- 22 files changed, 46 insertions(+), 20 deletions(-) Committed. Did you commit this patch? I only see the ChangeLog files have been updated by your commit. Am I missing something? Somehow only the ChangeLogs, which required manual editing, were marked to be added. I'll add the other files. -- Michael Eager
Re: [PATCH v6 1/1] gcc: config: microblaze: fix cpu version check
On 10/30/23 10:02, Neal Frager wrote: The MICROBLAZE_VERSION_COMPARE was incorrectly using strcasecmp instead of strverscmp to check the mcpu version against feature options. By simply changing the define to use strverscmp, the new version 10.0 is treated correctly as a higher version than previous versions. Signed-off-by: Neal Frager Added to commit message; Fix incorrect warning with -mcpu=10.0: warning: '-mxl-multiply-high' can be used only with '-mcpu=v6.00.a' or greater --- V1->V2: - No need to create a new microblaze specific version check routine as strverscmp is the correct solution. V2->V3: - Changed mcpu define for microblaze isa testsuite examples. V3->V4: - Added ChangeLog V4->V5: - Added testsuite ChangeLog V5->V6: - Updated testsuite ChangeLog to include all files --- gcc/ChangeLog | 4 gcc/config/microblaze/microblaze.cc | 2 +- gcc/testsuite/ChangeLog | 22 +++ .../gcc.target/microblaze/isa/bshift.c| 2 +- gcc/testsuite/gcc.target/microblaze/isa/div.c | 2 +- .../gcc.target/microblaze/isa/fcmp1.c | 2 +- .../gcc.target/microblaze/isa/fcmp2.c | 2 +- .../gcc.target/microblaze/isa/fcmp3.c | 2 +- .../gcc.target/microblaze/isa/fcmp4.c | 2 +- .../gcc.target/microblaze/isa/fcvt.c | 2 +- .../gcc.target/microblaze/isa/float.c | 2 +- .../gcc.target/microblaze/isa/fsqrt.c | 2 +- .../microblaze/isa/mul-bshift-pcmp.c | 2 +- .../gcc.target/microblaze/isa/mul-bshift.c| 2 +- gcc/testsuite/gcc.target/microblaze/isa/mul.c | 2 +- .../microblaze/isa/mulh-bshift-pcmp.c | 2 +- .../gcc.target/microblaze/isa/mulh.c | 2 +- .../gcc.target/microblaze/isa/nofcmp.c| 2 +- .../gcc.target/microblaze/isa/nofloat.c | 2 +- .../gcc.target/microblaze/isa/pcmp.c | 2 +- .../gcc.target/microblaze/isa/vanilla.c | 2 +- .../gcc.target/microblaze/microblaze.exp | 2 +- 22 files changed, 46 insertions(+), 20 deletions(-) Committed. -- Michael Eager
Re: [PATCH v5 1/1] gcc: config: microblaze: fix cpu version check
t-mul
-mxl-pattern-compare -mxl-multiply-high" } */
volatile int m1, m2, m3;
volatile unsigned int u1, u2, u3;
diff --git a/gcc/testsuite/gcc.target/microblaze/isa/mulh.c
b/gcc/testsuite/gcc.target/microblaze/isa/mulh.c
index 6e0cc3ac470..da28e8c4d1e 100644
--- a/gcc/testsuite/gcc.target/microblaze/isa/mulh.c
+++ b/gcc/testsuite/gcc.target/microblaze/isa/mulh.c
@@ -1,4 +1,4 @@
-/* { dg-options "-O3 -mcpu=v6.00.a -mno-xl-soft-mul -mxl-multiply-high" } */
+/* { dg-options "-O3 -mcpu=v10.0 -mno-xl-soft-mul -mxl-multiply-high" } */
volatile int m1, m2, m3;
volatile unsigned int u1, u2, u3;
diff --git a/gcc/testsuite/gcc.target/microblaze/isa/nofcmp.c
b/gcc/testsuite/gcc.target/microblaze/isa/nofcmp.c
index ebfb170ecee..86910fc347a 100644
--- a/gcc/testsuite/gcc.target/microblaze/isa/nofcmp.c
+++ b/gcc/testsuite/gcc.target/microblaze/isa/nofcmp.c
@@ -1,4 +1,4 @@
-/* { dg-options "-O3 -mcpu=v6.00.a " } */
+/* { dg-options "-O3 -mcpu=v10.0" } */
volatile float f1, f2, f3;
diff --git a/gcc/testsuite/gcc.target/microblaze/isa/nofloat.c b/gcc/testsuite/gcc.target/microblaze/isa/nofloat.c
index 647da3cfe24..b1f0268715d 100644
--- a/gcc/testsuite/gcc.target/microblaze/isa/nofloat.c
+++ b/gcc/testsuite/gcc.target/microblaze/isa/nofloat.c
@@ -1,4 +1,4 @@
-/* { dg-options "-O3 -mcpu=v6.00.a -msoft-float" } */
+/* { dg-options "-O3 -mcpu=v10.0 -msoft-float" } */
volatile float f1, f2, f3;
diff --git a/gcc/testsuite/gcc.target/microblaze/isa/pcmp.c b/gcc/testsuite/gcc.target/microblaze/isa/pcmp.c
index aea79572103..d9e5793f6f5 100644
--- a/gcc/testsuite/gcc.target/microblaze/isa/pcmp.c
+++ b/gcc/testsuite/gcc.target/microblaze/isa/pcmp.c
@@ -1,4 +1,4 @@
-/* { dg-options "-O3 -mcpu=v6.00.a -mxl-pattern-compare" } */
+/* { dg-options "-O3 -mcpu=v10.0 -mxl-pattern-compare" } */
volatile int m1, m2, m3;
volatile long l1, l2;
diff --git a/gcc/testsuite/gcc.target/microblaze/isa/vanilla.c
b/gcc/testsuite/gcc.target/microblaze/isa/vanilla.c
index 1d6ba807b12..35824b6d077 100644
--- a/gcc/testsuite/gcc.target/microblaze/isa/vanilla.c
+++ b/gcc/testsuite/gcc.target/microblaze/isa/vanilla.c
@@ -1,4 +1,4 @@
-/* { dg-options "-O3 -mcpu=v6.00.a -mcpu=v6.00.a" } */
+/* { dg-options "-O3 -mcpu=v10.0" } */
volatile int m1, m2, m3;
volatile long l1, l2;
diff --git a/gcc/testsuite/gcc.target/microblaze/microblaze.exp
b/gcc/testsuite/gcc.target/microblaze/microblaze.exp
index 1c7b0e23353..33979ae5e42 100644
--- a/gcc/testsuite/gcc.target/microblaze/microblaze.exp
+++ b/gcc/testsuite/gcc.target/microblaze/microblaze.exp
@@ -49,7 +49,7 @@ dg-runtest [lsort [glob -nocomplain
$srcdir/$subdir/isa/*.\[cSi\]]] \
${default_c_flags} ""
gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/others/*.\[cSi\]]] \
-"" "-mcpu=v6.00.a"
+"" "-mcpu=v10.0"
# All done.
--
Michael Eager
Re: [PATCH v4 1/1] gcc: config: microblaze: fix cpu version check
On 10/26/23 13:37, Neal Frager wrote:
The MICROBLAZE_VERSION_COMPARE was incorrectly using strcasecmp
instead of strverscmp to check the mcpu version against feature
options. By simply changing the define to use strverscmp,
the new version 10.0 is treated correctly as a higher version
than previous versions.
Signed-off-by: Neal Frager
---
V1->V2:
- No need to create a new microblaze specific version check
routine as strverscmp is the correct solution.
V2->V3:
- Changed mcpu define for microblaze isa testsuite examples.
V3->V4:
- Added ChangeLog
---
gcc/ChangeLog | 4
gcc/config/microblaze/microblaze.cc| 2 +-
gcc/testsuite/gcc.target/microblaze/isa/bshift.c | 2 +-
gcc/testsuite/gcc.target/microblaze/isa/div.c | 2 +-
gcc/testsuite/gcc.target/microblaze/isa/fcmp1.c| 2 +-
gcc/testsuite/gcc.target/microblaze/isa/fcmp2.c| 2 +-
gcc/testsuite/gcc.target/microblaze/isa/fcmp3.c| 2 +-
gcc/testsuite/gcc.target/microblaze/isa/fcmp4.c| 2 +-
gcc/testsuite/gcc.target/microblaze/isa/fcvt.c | 2 +-
gcc/testsuite/gcc.target/microblaze/isa/float.c| 2 +-
gcc/testsuite/gcc.target/microblaze/isa/fsqrt.c| 2 +-
gcc/testsuite/gcc.target/microblaze/isa/mul-bshift-pcmp.c | 2 +-
gcc/testsuite/gcc.target/microblaze/isa/mul-bshift.c | 2 +-
gcc/testsuite/gcc.target/microblaze/isa/mul.c | 2 +-
gcc/testsuite/gcc.target/microblaze/isa/mulh-bshift-pcmp.c | 2 +-
gcc/testsuite/gcc.target/microblaze/isa/mulh.c | 2 +-
gcc/testsuite/gcc.target/microblaze/isa/nofcmp.c | 2 +-
gcc/testsuite/gcc.target/microblaze/isa/nofloat.c | 2 +-
gcc/testsuite/gcc.target/microblaze/isa/pcmp.c | 2 +-
gcc/testsuite/gcc.target/microblaze/isa/vanilla.c | 2 +-
gcc/testsuite/gcc.target/microblaze/microblaze.exp | 2 +-
21 files changed, 24 insertions(+), 20 deletions(-)
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index d50cd42a7d4..d5fee35bda4 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,7 @@
+2023-10-26 Neal Frager
+
+ * config/microblaze/microblaze.cc: Fix mcpu version check.
gcc/testsuite/ChangeLog?
+
2023-10-25 Iain Sandoe
* config/darwin.cc (darwin_override_options): Handle fPIE.
diff --git a/gcc/config/microblaze/microblaze.cc
b/gcc/config/microblaze/microblaze.cc
index c9f6c4198cf..60ad55120d2 100644
--- a/gcc/config/microblaze/microblaze.cc
+++ b/gcc/config/microblaze/microblaze.cc
@@ -56,7 +56,7 @@
/* This file should be included last. */
#include "target-def.h"
-#define MICROBLAZE_VERSION_COMPARE(VA,VB) strcasecmp (VA, VB)
+#define MICROBLAZE_VERSION_COMPARE(VA,VB) strverscmp (VA, VB)
/* Classifies an address.
diff --git a/gcc/testsuite/gcc.target/microblaze/isa/bshift.c b/gcc/testsuite/gcc.target/microblaze/isa/bshift.c
index 64cf1e2e59e..664586bff9f 100644
--- a/gcc/testsuite/gcc.target/microblaze/isa/bshift.c
+++ b/gcc/testsuite/gcc.target/microblaze/isa/bshift.c
@@ -1,4 +1,4 @@
-/* { dg-options "-O3 -mcpu=v6.00.a -mxl-barrel-shift" } */
+/* { dg-options "-O3 -mcpu=v10.0 -mxl-barrel-shift" } */
volatile int m1, m2, m3;
volatile unsigned int u1, u2, u3;
diff --git a/gcc/testsuite/gcc.target/microblaze/isa/div.c
b/gcc/testsuite/gcc.target/microblaze/isa/div.c
index 25ee42ce5c8..783e7c0f684 100644
--- a/gcc/testsuite/gcc.target/microblaze/isa/div.c
+++ b/gcc/testsuite/gcc.target/microblaze/isa/div.c
@@ -1,4 +1,4 @@
-/* { dg-options "-O3 -mcpu=v6.00.a -mno-xl-soft-div" } */
+/* { dg-options "-O3 -mcpu=v10.0 -mno-xl-soft-div" } */
volatile int m1, m2, m3;
volatile long l1, l2;
diff --git a/gcc/testsuite/gcc.target/microblaze/isa/fcmp1.c
b/gcc/testsuite/gcc.target/microblaze/isa/fcmp1.c
index 4041a241391..b6202e168d6 100644
--- a/gcc/testsuite/gcc.target/microblaze/isa/fcmp1.c
+++ b/gcc/testsuite/gcc.target/microblaze/isa/fcmp1.c
@@ -1,4 +1,4 @@
-/* { dg-options "-O3 -mcpu=v6.00.a -mhard-float" } */
+/* { dg-options "-O3 -mcpu=v10.0 -mhard-float" } */
volatile float f1, f2, f3;
diff --git a/gcc/testsuite/gcc.target/microblaze/isa/fcmp2.c b/gcc/testsuite/gcc.target/microblaze/isa/fcmp2.c
index 3902b839db9..4386c6e6cc3 100644
--- a/gcc/testsuite/gcc.target/microblaze/isa/fcmp2.c
+++ b/gcc/testsuite/gcc.target/microblaze/isa/fcmp2.c
@@ -1,4 +1,4 @@
-/* { dg-options "-O3 -mcpu=v6.00.a -mhard-float" } */
+/* { dg-options "-O3 -mcpu=v10.0 -mhard-float" } */
volatile float f1, f2, f3;
diff --git a/gcc/testsuite/gcc.target/microblaze/isa/fcmp3.c b/gcc/testsuite/gcc.target/microblaze/isa/fcmp3.c
index 8555974dda5..b414e48fe1b 100644
--- a/gcc/testsuite/gcc.target/microblaze/isa/fcmp3.c
+++ b/gcc/testsuite/gcc.target/microblaze/isa/fcmp3.c
@@ -1,4 +1,4 @@
-/* { dg-options "-O3 -mcpu=v6.00.a -mhard-float" } */
+/* { dg-options "-O3 -mcpu=v10.0 -mhard-float" } */
Re: [PATCH v3 1/1] gcc: config: microblaze: fix cpu version check
On 10/25/23 01:02, Neal Frager wrote: The MICROBLAZE_VERSION_COMPARE was incorrectly using strcasecmp instead of strverscmp to check the mcpu version against feature options. By simply changing the define to use strverscmp, the new version 10.0 is treated correctly as a higher version than previous versions. Signed-off-by: Neal Frager --- V1->V2: - No need to create a new microblaze specific version check routine as strverscmp is the correct solution. V2->V3: - Changed mcpu define for microblaze isa testsuite examples. --- gcc/config/microblaze/microblaze.cc| 2 +- gcc/testsuite/gcc.target/microblaze/isa/bshift.c | 2 +- gcc/testsuite/gcc.target/microblaze/isa/div.c | 2 +- gcc/testsuite/gcc.target/microblaze/isa/fcmp1.c| 2 +- gcc/testsuite/gcc.target/microblaze/isa/fcmp2.c| 2 +- gcc/testsuite/gcc.target/microblaze/isa/fcmp3.c| 2 +- gcc/testsuite/gcc.target/microblaze/isa/fcmp4.c| 2 +- gcc/testsuite/gcc.target/microblaze/isa/fcvt.c | 2 +- gcc/testsuite/gcc.target/microblaze/isa/float.c| 2 +- gcc/testsuite/gcc.target/microblaze/isa/fsqrt.c| 2 +- gcc/testsuite/gcc.target/microblaze/isa/mul-bshift-pcmp.c | 2 +- gcc/testsuite/gcc.target/microblaze/isa/mul-bshift.c | 2 +- gcc/testsuite/gcc.target/microblaze/isa/mul.c | 2 +- gcc/testsuite/gcc.target/microblaze/isa/mulh-bshift-pcmp.c | 2 +- gcc/testsuite/gcc.target/microblaze/isa/mulh.c | 2 +- gcc/testsuite/gcc.target/microblaze/isa/nofcmp.c | 2 +- gcc/testsuite/gcc.target/microblaze/isa/nofloat.c | 2 +- gcc/testsuite/gcc.target/microblaze/isa/pcmp.c | 2 +- gcc/testsuite/gcc.target/microblaze/isa/vanilla.c | 2 +- gcc/testsuite/gcc.target/microblaze/microblaze.exp | 2 +- 20 files changed, 20 insertions(+), 20 deletions(-) Only two test cases actually test this fix. Please add a ChangeLog update for these changes. -- Michael Eager
Re: [PATCH v1 1/1] gcc: config: microblaze: fix cpu version check
On 10/24/23 00:01, Frager, Neal wrote:
There is a microblaze cpu version 10.0 included in versal. If the
minor version is only a single digit, then the version comparison
will fail as version 10.0 will appear as 100 compared to version
6.00 or 8.30 which will calculate to values 600 and 830.
The issue can be seen when using the '-mcpu=10.0' option.
With this fix, versions with a single digit minor number such as
10.0 will be calculated as greater than versions with a smaller
major version number, but with two minor version digits.
By applying this fix, several incorrect warning messages will no
longer be printed when building the versal plm application, such as
the warning message below:
warning: '-mxl-multiply-high' can be used only with '-mcpu=v6.00.a'
or greater
Signed-off-by: Neal Frager
---
gcc/config/microblaze/microblaze.cc | 164 +---
1 file changed, 76 insertions(+), 88 deletions(-)
Please add a test case.
--
Michael Eager
Hi Michael,
Would you mind helping me understand how to make a gcc test case for this patch?
This patch does not change the resulting binaries of a microblaze gcc build.
The output will be the same with our without the patch, so I do not having
anything in the binary itself to verify.
All that happens is false warning messages will not be printed when building
with ‘-mcpu=10.0’. Is there a way to test for warning messages?
In any case, please do not commit v1 of this patch. I am going to work on
making a v2 based on Mark’s feedback.
You can create a test case which passes the -mcpu=10.0 and other options to GCC
and verify that the message is not generated after the patch is applied.
You can make all GCC warnings into errors with the "-Werror" option.
This means that the compile will fail if the warning is issued.
Take a look at gcc/testsuite/gcc.target/aarch64/bti-1.c for an example of using { dg-options
"" } to specify command line options.
There is a test suite option (dg-warning) which checks that a particular source
line generates a warning message, but it isn't clear whether is is possible to
check that a warning is not issued.
Hi Michael,
Thanks to Mark Hatle's feedback, we have a much simpler solution to the problem.
The following change is actually all that is necessary. Since we are just
moving from
strcasecmp to strverscmp, does v2 of the patch need to have a test case to go
with it?
-#define MICROBLAZE_VERSION_COMPARE(VA,VB) strcasecmp (VA, VB)
+#define MICROBLAZE_VERSION_COMPARE(VA,VB) strverscmp (VA, VB)
I assume there are already test cases that verify that strverscmp works
correctly?
Still need a test case to verify this fix.
--
Michael Eager
Re: [PATCH v1 1/1] gcc: config: microblaze: fix cpu version check
On 10/23/23 11:37, Frager, Neal wrote:
Le 23 oct. 2023 à 18:40, Michael Eager a écrit :
On 10/22/23 22:48, Neal Frager wrote:
There is a microblaze cpu version 10.0 included in versal. If the
minor version is only a single digit, then the version comparison
will fail as version 10.0 will appear as 100 compared to version
6.00 or 8.30 which will calculate to values 600 and 830.
The issue can be seen when using the '-mcpu=10.0' option.
With this fix, versions with a single digit minor number such as
10.0 will be calculated as greater than versions with a smaller
major version number, but with two minor version digits.
By applying this fix, several incorrect warning messages will no
longer be printed when building the versal plm application, such
as the warning message below:
warning: '-mxl-multiply-high' can be used only with '-mcpu=v6.00.a' or greater
Signed-off-by: Neal Frager
---
gcc/config/microblaze/microblaze.cc | 164 +---
1 file changed, 76 insertions(+), 88 deletions(-)
Please add a test case.
--
Michael Eager
Hi Michael,
Would you mind helping me understand how to make a gcc test case for this patch?
This patch does not change the resulting binaries of a microblaze gcc build.
The output will be the same with our without the patch, so I do not having
anything in the binary itself to verify.
All that happens is false warning messages will not be printed when building
with ‘-mcpu=10.0’. Is there a way to test for warning messages?
In any case, please do not commit v1 of this patch. I am going to work on
making a v2 based on Mark’s feedback.
You can create a test case which passes the -mcpu=10.0 and other options
to GCC and verify that the message is not generated after the patch is
applied.
You can make all GCC warnings into errors with the "-Werror" option.
This means that the compile will fail if the warning is issued.
Take a look at gcc/testsuite/gcc.target/aarch64/bti-1.c for an example
of using { dg-options "" } to specify command line options.
There is a test suite option (dg-warning) which checks that a particular
source line generates a warning message, but it isn't clear whether is
is possible to check that a warning is not issued.
--
Michael Eager
Re: [PATCH v1 1/1] gcc: config: microblaze: fix cpu version check
On 10/22/23 22:48, Neal Frager wrote: There is a microblaze cpu version 10.0 included in versal. If the minor version is only a single digit, then the version comparison will fail as version 10.0 will appear as 100 compared to version 6.00 or 8.30 which will calculate to values 600 and 830. The issue can be seen when using the '-mcpu=10.0' option. With this fix, versions with a single digit minor number such as 10.0 will be calculated as greater than versions with a smaller major version number, but with two minor version digits. By applying this fix, several incorrect warning messages will no longer be printed when building the versal plm application, such as the warning message below: warning: '-mxl-multiply-high' can be used only with '-mcpu=v6.00.a' or greater Signed-off-by: Neal Frager --- gcc/config/microblaze/microblaze.cc | 164 +--- 1 file changed, 76 insertions(+), 88 deletions(-) Please add a test case. -- Michael Eager
DDD-3.4.0 Debbugger GUI released
DDD, the DATA DISPLAY DEBUGGER, version 3.4.0 has been released. DDD is a graphical front end for GDB (and other debuggers) with an intuitive interface and an interactive graphical display for data. This release, the first since 2009, has been updated to support current OS and library versions, improve the build process, fix a number of bugs as well as make a number of enhancements. DDD's maintainers are Stefan Eickler and Michael Eager. Please send questions or comments to mailto:d...@gnu.org. Information about DDD can be found on the DDD project page: https://www.gnu.org/software/ddd/ -- Michael Eager
Re: MicroBlaze symver attribute support
On 2/20/23 06:54, Joel Sherrill wrote: On Mon, Feb 20, 2023 at 7:56 AM Vincent Fazio via Gcc <mailto:gcc@gcc.gnu.org>> wrote: Michael, all, Regarding: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101766 <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101766> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102110 <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102110> If I understand correctly, since the GCC MicroBlaze targets generate ELF images, it would seem there's no technical reason why `__attribute__ ((symver ...))` cannot be supported? The issue seems to be that the MicroBlaze targets in config.gcc do not include elfos.h https://github.com/gcc-mirror/gcc/blob/master/gcc/config.gcc#L2369 <https://github.com/gcc-mirror/gcc/blob/master/gcc/config.gcc#L2369> Which defines `ASM_OUTPUT_SYMVER_DIRECTIVE` https://github.com/gcc-mirror/gcc/blob/master/gcc/config/elfos.h#L259 <https://github.com/gcc-mirror/gcc/blob/master/gcc/config/elfos.h#L259> Which is necessary for https://github.com/gcc-mirror/gcc/blob/master/gcc/varasm.cc#L6260 <https://github.com/gcc-mirror/gcc/blob/master/gcc/varasm.cc#L6260> Changing the targets to include elfos.h leads to a number of conflicts with the target specific header (config/microblaze/microblaze.h). Should `ASM_OUTPUT_SYMVER_DIRECTIVE` be copied into config/microblaze/microblaze.h or should the MicroBlaze targets and header be reworked to support elfos.h? IMO reworked to include elfos.h. During the RTEMS port, we noticed that some quirks on the microblaze gcc because it isn't using elfos.h. This likely will mean it will be updated as a side-effect of normal GCC maintenance instead of being an odd singleton which doesn't share. I'm asking because I've seen a number of projects run into this issue (xz, elfutils, libfuse, libkcapi, cryptsetup). And RTEMS. --joel Joel -- do you have a patch to add elfos.h to MicroBlaze? -- Michael Eager
[PATCH] Microblaze: Fix uninitialized variable warnings
The attached patch corrects a couple uninitialized variable warnings.
The variables are initialized to NULL and tested for this, calling
gcc_unreachable(). Replace other calls to abort() for with
gcc_unreachable().
Thanks to Jan-Benedict Glaw for bringing this to my attention.
** I'm receiving a "service not enabled" error when I push.
** Can someone apply this patch while I resolve this issue?
--
Michael EagerFrom a0fd2e9baa51e85f61cebd6e78bef8b5c55199b5 Mon Sep 17 00:00:00 2001
From: Michael Eager
Date: Thu, 20 Oct 2022 09:33:13 -0700
Subject: [PATCH] Fix uninitialized variable warnings
gcc/ChangeLog:
* gcc/config/microblaze/microblaze.cc
(microblaze_legitimize_address): Initialize 'reg' to NULL, check for NULL.
(microblaze_address_insns): Replace abort() with gcc_unreachable().
(print_operand_address): Same.
(microblaze_expand_move): Initialize 'p1' to NULL, check for NULL.
(get_branch_target): Replace abort() with gcc_unreachable().
---
gcc/ChangeLog | 9 +
gcc/config/microblaze/microblaze.cc | 19 ++-
2 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 7a50293c780..8271fafe033 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,12 @@
+2022-10-20 Michael Eager
+
+ * gcc/config/microblaze/microblaze.cc
+ (microblaze_legitimize_address): Initialize 'reg' to NULL, check for NULL.
+ (microblaze_address_insns): Replace abort() with gcc_unreachable().
+ (print_operand_address): Same.
+ (microblaze_expand_move): Initialize 'p1' to NULL, check for NULL.
+ (get_branch_target): Replace abort() with gcc_unreachable().
+
2022-10-19 Aldy Hernandez
* range-op-float.cc (build_le): Document result.
diff --git a/gcc/config/microblaze/microblaze.cc b/gcc/config/microblaze/microblaze.cc
index 8fcca1829f6..9290a1f3958 100644
--- a/gcc/config/microblaze/microblaze.cc
+++ b/gcc/config/microblaze/microblaze.cc
@@ -1103,7 +1103,7 @@ microblaze_legitimize_address (rtx x, rtx oldx ATTRIBUTE_UNUSED,
if (GET_CODE (xinsn) == SYMBOL_REF)
{
- rtx reg;
+ rtx reg = NULL;
if (microblaze_tls_symbol_p(xinsn))
{
reg = microblaze_legitimize_tls_address (xinsn, NULL_RTX);
@@ -1133,6 +1133,11 @@ microblaze_legitimize_address (rtx x, rtx oldx ATTRIBUTE_UNUSED,
reg = pic_ref;
}
}
+ else
+ {
+ /* This should never happen. */
+ gcc_unreachable ();
+ }
return reg;
}
@@ -1474,7 +1479,7 @@ microblaze_address_insns (rtx x, machine_mode mode)
case TLS_DTPREL:
return 1;
default :
- abort();
+ gcc_unreachable ();
}
default:
break;
@@ -2624,7 +2629,7 @@ print_operand_address (FILE * file, rtx addr)
fputs ("@TLSDTPREL", file);
break;
default :
- abort();
+ gcc_unreachable ();
break;
}
}
@@ -3413,7 +3418,7 @@ microblaze_expand_move (machine_mode mode, rtx operands[])
}
if (GET_CODE (op1) == PLUS && GET_CODE (XEXP (op1,1)) == CONST)
{
- rtx p0, p1, result, temp;
+ rtx p0, p1 = NULL, result, temp;
p0 = XEXP (XEXP (op1,1), 0);
@@ -3423,6 +3428,10 @@ microblaze_expand_move (machine_mode mode, rtx operands[])
p0 = XEXP (p0, 0);
}
+ /* This should never happen. */
+ if (p1 == NULL)
+ gcc_unreachable ();
+
if (GET_CODE (p0) == UNSPEC && GET_CODE (p1) == CONST_INT
&& flag_pic && TARGET_PIC_DATA_TEXT_REL)
{
@@ -3799,7 +3808,7 @@ get_branch_target (rtx branch)
if (GET_CODE (call) == SET)
call = SET_SRC (call);
if (GET_CODE (call) != CALL)
-abort ();
+ gcc_unreachable ();
return XEXP (XEXP (call, 0), 0);
}
--
2.31.1
[PATCH] Fix uninitialized variable warnings
The attached patch corrects a couple uninitialized variable warnings.
The variables are initialized to NULL and tests for this, calling
gcc_unreachable(). Replace other calls to abort() for with
gcc_unreachable().
Thanks to Jan-Benedict Glaw for bringing this to my attention.
** I'm receiving a "service not enabled" error when I push.
** Can someone apply this patch while I resolve this issue?
--
Michael EagerFrom a0fd2e9baa51e85f61cebd6e78bef8b5c55199b5 Mon Sep 17 00:00:00 2001
From: Michael Eager
Date: Thu, 20 Oct 2022 09:33:13 -0700
Subject: [PATCH] Fix uninitialized variable warnings
gcc/ChangeLog:
* gcc/config/microblaze/microblaze.cc
(microblaze_legitimize_address): Initialize 'reg' to NULL, check for NULL.
(microblaze_address_insns): Replace abort() with gcc_unreachable().
(print_operand_address): Same.
(microblaze_expand_move): Initialize 'p1' to NULL, check for NULL.
(get_branch_target): Replace abort() with gcc_unreachable().
---
gcc/ChangeLog | 9 +
gcc/config/microblaze/microblaze.cc | 19 ++-
2 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 7a50293c780..8271fafe033 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,12 @@
+2022-10-20 Michael Eager
+
+ * gcc/config/microblaze/microblaze.cc
+ (microblaze_legitimize_address): Initialize 'reg' to NULL, check for NULL.
+ (microblaze_address_insns): Replace abort() with gcc_unreachable().
+ (print_operand_address): Same.
+ (microblaze_expand_move): Initialize 'p1' to NULL, check for NULL.
+ (get_branch_target): Replace abort() with gcc_unreachable().
+
2022-10-19 Aldy Hernandez
* range-op-float.cc (build_le): Document result.
diff --git a/gcc/config/microblaze/microblaze.cc b/gcc/config/microblaze/microblaze.cc
index 8fcca1829f6..9290a1f3958 100644
--- a/gcc/config/microblaze/microblaze.cc
+++ b/gcc/config/microblaze/microblaze.cc
@@ -1103,7 +1103,7 @@ microblaze_legitimize_address (rtx x, rtx oldx ATTRIBUTE_UNUSED,
if (GET_CODE (xinsn) == SYMBOL_REF)
{
- rtx reg;
+ rtx reg = NULL;
if (microblaze_tls_symbol_p(xinsn))
{
reg = microblaze_legitimize_tls_address (xinsn, NULL_RTX);
@@ -1133,6 +1133,11 @@ microblaze_legitimize_address (rtx x, rtx oldx ATTRIBUTE_UNUSED,
reg = pic_ref;
}
}
+ else
+ {
+ /* This should never happen. */
+ gcc_unreachable ();
+ }
return reg;
}
@@ -1474,7 +1479,7 @@ microblaze_address_insns (rtx x, machine_mode mode)
case TLS_DTPREL:
return 1;
default :
- abort();
+ gcc_unreachable ();
}
default:
break;
@@ -2624,7 +2629,7 @@ print_operand_address (FILE * file, rtx addr)
fputs ("@TLSDTPREL", file);
break;
default :
- abort();
+ gcc_unreachable ();
break;
}
}
@@ -3413,7 +3418,7 @@ microblaze_expand_move (machine_mode mode, rtx operands[])
}
if (GET_CODE (op1) == PLUS && GET_CODE (XEXP (op1,1)) == CONST)
{
- rtx p0, p1, result, temp;
+ rtx p0, p1 = NULL, result, temp;
p0 = XEXP (XEXP (op1,1), 0);
@@ -3423,6 +3428,10 @@ microblaze_expand_move (machine_mode mode, rtx operands[])
p0 = XEXP (p0, 0);
}
+ /* This should never happen. */
+ if (p1 == NULL)
+ gcc_unreachable ();
+
if (GET_CODE (p0) == UNSPEC && GET_CODE (p1) == CONST_INT
&& flag_pic && TARGET_PIC_DATA_TEXT_REL)
{
@@ -3799,7 +3808,7 @@ get_branch_target (rtx branch)
if (GET_CODE (call) == SET)
call = SET_SRC (call);
if (GET_CODE (call) != CALL)
-abort ();
+ gcc_unreachable ();
return XEXP (XEXP (call, 0), 0);
}
--
2.31.1
Re: Proper Place for builtin_define(__ELF__)
On 7/21/21 5:22 PM, Joel Sherrill wrote: On Wed, Jul 21, 2021, 7:12 PM Michael Eager <mailto:ea...@eagercon.com>> wrote: On 7/21/21 2:28 PM, Joel Sherrill wrote: > Hi > > We are in the process of porting RTEMS to the Microblaze and gcc does > not have __ELF__ as a predefine. In looking around at where to add it, > it looks like there are multiple ways to do it. We see variations on > the following patterns: > > + dbxelf.h > + OS specific header in config/ > + Arch/OS specific header > > Integrating dbxelf.h into the microblaze seems risky for one simple > builtin_define(). Adding it to config/microblaze/rtems.h won't address > the microblaze-elf target. > > A suggestion on where to add the builtin_predefine is appreciated. There are very few defines for __ELF__ in the GCC target files. Many targets include dbxelf.h from the config.gcc script. There are 130 references to that file there. That seems to be where most architectures get it. AFAIK, no one has ever tried to build microblaze to generate stabs, and I can't see a good reason why anyone would. Including dbxelf.h seems wrong. I don't have an answer why other arch's do that. Why don't you put this in rtems.h? That's ok for a hack but we haven't had to do that on the other ports so it seems wrong. Yep. I didn't mention but without this defined the cdefs.h file in newlib produces incorrect macro definitions for at the weak_reference macro. Alternately, you might put it in microblaze-s.c, wrapped with #ifdef OBJECT_FORMAT_ELF/#endif. Ok. This should fix it for microblaze-elf also. ARM does something which looks screwy to me. Instead of defining __ELF__, they pass -D__ELF__ on the CPP command line. Thanks. --joel -- Michael Eager -- Michael Eager
Re: Proper Place for builtin_define(__ELF__)
On 7/21/21 2:28 PM, Joel Sherrill wrote: Hi We are in the process of porting RTEMS to the Microblaze and gcc does not have __ELF__ as a predefine. In looking around at where to add it, it looks like there are multiple ways to do it. We see variations on the following patterns: + dbxelf.h + OS specific header in config/ + Arch/OS specific header Integrating dbxelf.h into the microblaze seems risky for one simple builtin_define(). Adding it to config/microblaze/rtems.h won't address the microblaze-elf target. A suggestion on where to add the builtin_predefine is appreciated. There are very few defines for __ELF__ in the GCC target files. Why don't you put this in rtems.h? Alternately, you might put it in microblaze-s.c, wrapped with #ifdef OBJECT_FORMAT_ELF/#endif. -- Michael Eager
DWARF Committee resumes meetings
The DWARF Debugging Standard Committee (http://dwarfstd.org) is planning to resume meetings starting in February. The DWARF debugging file format, as you likely know, is widely used by compilers and debuggers to support source level debugging. The Committee released Version 5 of the DWARF Standard in 2017 and will start work on Version 6. The committee consists of more than 20 committee members from a broad cross-section of the compiler and debugger development community, representing a dozen companies and including a number of independent developers, working on both proprietary and open source products. A list of the current proposals or issues can be found on the Open Issues web page: http://dwarfstd.org/Issues.php. If you would like to submit a comment, proposal, or issue, go to the Public Comment page: http://dwarfstd.org/Comment.php. If you are interested in participating on the DWARF Committee please contact me privately. -- Michael Eager, DWARF Committee Chair
Re: [Patch, microblaze]: Correct the const high double immediate value
On 11/8/20 9:43 PM, Nagaraju Mekala wrote:
diff --git a/gcc/config/microblaze/microblaze.c
b/gcc/config/microblaze/microblaze.c
index a0f81b7..d9341ec 100644
--- a/gcc/config/microblaze/microblaze.c
+++ b/gcc/config/microblaze/microblaze.c
@@ -2440,15 +2440,18 @@ print_operand (FILE * file, rtx op, int letter)
else if (letter == 'h' || letter == 'j')
{
- long val[2];
+ long val[2], l[2];
if (code == CONST_DOUBLE)
{
if (GET_MODE (op) == DFmode)
REAL_VALUE_TO_TARGET_DOUBLE (*CONST_DOUBLE_REAL_VALUE (op),
val);
else
{
- val[0] = CONST_DOUBLE_HIGH (op);
- val[1] = CONST_DOUBLE_LOW (op);
+ REAL_VALUE_TYPE rv;
+ REAL_VALUE_FROM_CONST_DOUBLE (rv, op);
REAL_VALUE_FROM_CONST_DOUBLE was removed from real.h in 2015.
Use CONST_DOUBLE_REAL_VALUE.
+ REAL_VALUE_TO_TARGET_DOUBLE (rv, l);
+ val[1] = l[WORDS_BIG_ENDIAN == 0];
+ val[0] = l[WORDS_BIG_ENDIAN != 0];
}
}
else if (code == CONST_INT)
diff --git a/gcc/testsuite/gcc.target/microblaze/long.c
b/gcc/testsuite/gcc.target/microblaze/long.c
new file mode 100644
index 000..4d45186
--- /dev/null
+++ b/gcc/testsuite/gcc.target/microblaze/long.c
@@ -0,0 +1,10 @@
+/* { dg-options "-O0" } */
+#define BASEADDR 0xF000ULL
+int main ()
+{
+ unsigned long long start;
+ start = (unsigned long long) BASEADDR;
+ return 0;
+}
+/* { dg-final { scan-assembler
"addik\tr(\[0-9]\|\[1-2]\[0-9]\|3\[0-1]),r0,0x" } } */
+/* { dg-final { scan-assembler
"addik\tr(\[0-9]\|\[1-2]\[0-9]\|3\[0-1]),r0,0xf000" } } */
It looks like this test case will pass without the patch. The code
generated before applying the patch is
addik r4,r0,0x
addik r5,r0,0xf000 #li => la
Can you provide a test case which fails without the patch but passes
with the patch?
--
Michael Eager
Re: [PATCH]Microblaze: Fixed missing save of r18 in fast_interrupt.
OK to apply.
On 4/4/20 2:18 AM, Nagaraju Mekala wrote:
Hello All,
Fixed missing save of r18 in fast_interrupt.
Register 18 is used as a clobber register, and must be stored when entering a
fast_interrupt. Before this fix, register 18 was only saved if it was used
directly in the interrupt function.
However, if the fast_interrupt function called a function that used r18, the register would not be saved, and thus be mangled upon returning from the interrupt.
Changelog
2020-04-04 Klaus Petersen
* gcc/config/microblaze/microblaze.c: Check for fast_interrupt in
microblaze_must_save_register.
Signed-off-by: Klaus Petersen
Signed-off-by :Nagaraju Mekala
diff --git a/gcc/config/microblaze/microblaze.c
b/gcc/config/microblaze/microblaze.c
index b4754b1..67e393d 100644
--- a/gcc/config/microblaze/microblaze.c
+++ b/gcc/config/microblaze/microblaze.c
@@ -2035,7 +2035,7 @@ microblaze_must_save_register (int regno)
{
if (df_regs_ever_live_p (regno)
|| regno == MB_ABI_MSR_SAVE_REG
- || (interrupt_handler
+ || ((interrupt_handler || fast_interrupt)
&& (regno == MB_ABI_ASM_TEMP_REGNUM
|| regno == MB_ABI_EXCEPTION_RETURN_ADDR_REGNUM)))
return 1;
Attached is the patch.
Thanks
Nagaraju
--
Michael Eagerea...@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306
Re: [PATCH]Microblaze: Modified trap instruction
OK to apply.
On 4/4/20 1:59 AM, Nagaraju Mekala wrote:
Hello All,
There is a bug in trap instruction generation.
Instead of "bri 0" instruction "brki r0, -1" was used, corrected it now.
ChangeLog:
2020-04-04 Nagaraju Mekala
* gcc/config/microblaze/microblaze.md
(trap): update in the pattern
* gcc/testsuite/gcc.target/microblaze/others/builtin-trap.c
(dg-final): update in the scan-assembler instruction
Signed-off-by :Nagaraju Mekala
diff --git a/gcc/config/microblaze/microblaze.md
b/gcc/config/microblaze/microblaze.md
index 1970cc6..7049acd 100644
--- a/gcc/config/microblaze/microblaze.md
+++ b/gcc/config/microblaze/microblaze.md
@@ -2303,7 +2303,7 @@
(define_insn "trap"
[(trap_if (const_int 1) (const_int 0))]
""
- "brki\tr0,-1"
+ "bri\t0"
[(set_attr "type" "trap")]
)
diff --git a/gcc/testsuite/gcc.target/microblaze/others/builtin-trap.c b/gcc/testsuite/gcc.target/microblaze/others/builtin-trap.c
index fdcde1f..580b4db 100644
--- a/gcc/testsuite/gcc.target/microblaze/others/builtin-trap.c
+++ b/gcc/testsuite/gcc.target/microblaze/others/builtin-trap.c
@@ -5,4 +5,4 @@ void trap ()
__builtin_trap ();
}
-/* { dg-final { scan-assembler "brki\tr0,-1" } } */
\ No newline at end of file
+/* { dg-final { scan-assembler "bri\t0" } } */
Attached is the patch.
Thanks
Nagaraju
--
Michael Eagerea...@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306
Re: [PATCH] microblaze: fix PR65649
On 11/15/19 2:07 AM, Szabolcs Nagy wrote: microblaze-linux-musl build fails without this. (This is a rebase of an earlier patch posted on bugzilla.) gcc/ChangeLog: 2019-11-15 Nick Clifton Szabolcs Nagy * config/microblaze/microblaze.c (print_operand): Print value as long. OK. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306
Re: Question about make_extraction() in combine.c
On 8/28/19 12:33 PM, Segher Boessenkool wrote: Hi! On Tue, Aug 27, 2019 at 09:37:59AM -0700, Michael Eager wrote: Combine is complex, but I don't think that target descriptions should conform to its behaviors; But they have to, in some ways. If combine writes something that can be written in multiple ways in some way X, then your machine description has to recognise X (perhaps in addition to other ways it can be written), or you will not get as much optimisation as you might like: some combine attempts will fail. combine should adapt to the target. How? By not making arbitrary restrictions on the instructions which a target can implement, simply to avoid a bug in a different target. The target has an instruction which can extract a bit (or bit field) from a word in memory. The code in combine prevents that instruction from being used without creating awkward workarounds. diff --git a/gcc/combine.c b/gcc/combine.c index 93bd3da26d7..fdc79ab7d3e 100644 --- a/gcc/combine.c +++ b/gcc/combine.c @@ -,17 +,7 @@ make_extraction (machine_mode mode, rtx inner, HOST_WIDE_INT pos, This patch is against some older version of combine.c? The line number is off by 70 or so. The patch was created last November. && partial_subreg_p (extraction_mode, mode)) extraction_mode = mode; And current trunk has here /* Punt if len is too large for extraction_mode. */ if (maybe_gt (len, GET_MODE_PRECISION (extraction_mode))) return NULL_RTX; (See r268913, https://gcc.gnu.org/ml/gcc-patches/2019-02/msg01140.html ). This patch seems unrelated. Does that fix your problem already? Is more needed? Is your patch removing some now-dead code? If you are asking, does my patch remove code that is no longer needed to fix the decades-old problem which it fixed (or hid)? I suspect it does. But this cannot be verified. There's no test case for the original problem, nor is it clear which architecture had the problem. -- Michael Eagerea...@eagerm.com 1960 Park Blvd., Palo Alto, CA 94306
Re: Question about make_extraction() in combine.c
On 8/20/19 4:07 PM, Jeff Law wrote:
On 11/20/18 4:39 PM, Michael Eager wrote:
On 11/20/2018 03:10 PM, Jeff Law wrote:
On 11/20/18 11:07 AM, Michael Eager wrote:
On 11/18/2018 08:14 AM, Jeff Law wrote:
On 11/18/18 8:44 AM, Michael Eager wrote:
On 11/16/18 14:50, Segher Boessenkool wrote:
Hi!
On Wed, Nov 14, 2018 at 11:22:58AM -0800, Michael Eager wrote:
The (mem:SI) is converted to (mem:QI).
The return from make_extract() is
(zero_extract:SI (mem:QI (reg/v/f:SI 50 [ gp ]))
(const_int 1 [0x1])
(const_int 2 [0x2]))
The target has an instruction pattern for zero_extract, but it
matches
SI values, not QI. So the instruction which implements a test of a
bit
flag is never generated.
The RTL documentation says:
If @var{loc} is in memory, its mode must be a single-byte
integer
mode.
But obviously various ports use other modes, too. I wonder if that
ever
worked well.
Thanks. I hadn't noticed this.
Does anyone have any idea why there is a restriction on the mode?
Other instruction patterns don't place arbitrary restriction on the
memory access mode.
Not offhand. BUt it's been around for a long time (at least since the
early 90s). I stumbled over it several times through the years. It
could well be an artifact of the m68k or the vax.
The internal RTL should not be dictating what the target arch can or
cannot implement. Reload should insert any needed conversions,
especially ones which narrow the size.
I have a patch which removes this conversion. Works for my target. I
built and ran 'make check' for x86 with no additional failures. I don't
have a VAX or M68K sitting around to test. :-)
I can do correctness tests for m68k via qemu. Essentially verifying it
still bootstraps:-) Just pass along a git formatted patch and I can
throw it into the tester.
Here's the patch. I'll edit the doc if this works for 68K.
So I'm finally coming back to this. The test certainly bootstraps on
the m68k and various other targets. It doesn't seem to cause any
regressions on the various targets that get tested.
I wandered the backends and uses of zero_extract with MEMs not in QImode
are actually relatively rare and not likely things that are being
exercised thoroughly. Ironically the vax seems to have the most of
these kinds of patterns.
After re-reviewing Paulo's changes from 2006 as well as an old thread
from the gcc-2 era, I'm even more concerned than I was before WRT this
change potentially allowing a read beyond the boundary of the original
object, particularly on targets that don't require strict alignment.
That could be fatal if the object happens to be at the end of a page.
The 1992 thread I suspect is what ultimately led to the note in the
manual which limits memory operands of a zero_extract to QImode. We
didn't have version control in those days, so I can't be 100% certain.
This kind of situation is rare, but not unheard of (I've tripped over
this kind of stuff myself).
I think we're ultimately better off fixing the target to better match
what combine is doing.
Thanks for investigating this. It looks like you did quite a bit of
archaeological research.
It seems to me that the only way that a read outside of the original
object bounds is if the size is expanded. There's code in combine.c to
insure that does not happen. I guess that an extv could be generated
with incorrect operands which would be outside the object bounds, but
that would be a bug.
I don't doubt that the QI restriction on zero_extract resolves some
long-ago bug report, but I suspect that it hides the bug rather than
fixes it. It's also possible that the actual bug has been fixed, but
the restriction has never been removed. Without a test case, it isn't
easy to do more than offer a conjecture.
Combine is complex, but I don't think that target descriptions should
conform to its behaviors; combine should adapt to the target.
(Incidentally, the target has a peephole optimization which is supposed
to do what combine should be doing in this case. It worked with GCC-4;
I don't recall why I never investigated why it isn't working with GCC-8.
Possibly, the error occurs before peephole optimization.)
As a practical matter, the patch seems to work without problem. (I've
attached it here again; I'd sent it previously in an off-list email.)
The target is not in the GCC tree, but in a separate public repo, and I
don't believe that it will ever be upstreamed, so there's no issue with
a future merge conflict. My project to upgrade the target to GCC-8 was
completed last year and I'm no longer maintaining the tool chain.
Thanks again for following up on this.
--
Michael Eagerea...@eagerm.com
1960 Park Blvd., Palo Alto, CA 94306
>From 7de0670b38c7b1cc3b805bdebe638357f384a9f8 Mon Sep 17 00:00:00 2001
From: Michael Eager
Date: Tue, 20 Nov 2018 15:37:10 -0800
Subject: [PATCH] Eliminate narrowing of zero_extract MEM reference
Combine (make_extract
Re: [PATCH 17/30] Changes to microblaze
OK On 6/25/19 1:22 PM, acsaw...@linux.ibm.com wrote: From: Aaron Sawdey * config/microblaze/microblaze.c: Change movmem to cpymem in comment. * config/microblaze/microblaze.md (movmemsi): Change name to cpymemsi. --- gcc/config/microblaze/microblaze.c | 2 +- gcc/config/microblaze/microblaze.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/gcc/config/microblaze/microblaze.c b/gcc/config/microblaze/microblaze.c index 947eef8..c2cbe3b 100644 --- a/gcc/config/microblaze/microblaze.c +++ b/gcc/config/microblaze/microblaze.c @@ -1250,7 +1250,7 @@ microblaze_block_move_loop (rtx dest, rtx src, HOST_WIDE_INT length) microblaze_block_move_straight (dest, src, leftover); } -/* Expand a movmemsi instruction. */ +/* Expand a cpymemsi instruction. */ bool microblaze_expand_block_move (rtx dest, rtx src, rtx length, rtx align_rtx) diff --git a/gcc/config/microblaze/microblaze.md b/gcc/config/microblaze/microblaze.md index 183afff..1509e43 100644 --- a/gcc/config/microblaze/microblaze.md +++ b/gcc/config/microblaze/microblaze.md @@ -1144,7 +1144,7 @@ ;; Argument 2 is the length ;; Argument 3 is the alignment -(define_expand "movmemsi" +(define_expand "cpymemsi" [(parallel [(set (match_operand:BLK 0 "general_operand") (match_operand:BLK 1 "general_operand")) (use (match_operand:SI 2 "")) -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306
Re: Gimpel lowering question.
On 11/29/18 10:28, Michael Eager wrote:
On 11/28/18 14:37, Andrew Pinski wrote:
On Wed, Nov 28, 2018 at 9:47 AM Michael Eager wrote:
On 11/28/18 09:10, Jeff Law wrote:
On 11/28/18 10:00 AM, Michael Eager wrote:
I have a small test case which generates poor quality code on my
target.
Here is the original:
if (cond1 == 2048 || cond2 == 8)
{
x = x + y;
}
return x;
This ends up generating a series of instructions to compute a flag
with
the result of the condition followed by a single compare with zero and
a jump. Better code would be two compares and two jumps.
The gimple is
_1 = cond1 == 2048;
_2 = cond2 == 8;
_3 = _1 | _2;
if (_3 != 0) goto ; else goto ;
...
so this poor code sequence is essentially a correct translation.
On MIPS, for the same test case, the gimple is different:
if (cond1 == 2048) goto ; else goto ;
:
if (cond2 == 8) goto ; else goto ;
:
...
which generates the better code sequence.
Can someone give me a pointer where to find where the lowering
pass decides to break up a condition into multiple tests? Is
there a target configuration option which I have overlooked or
maybe set incorrectly?
BRANCH_COST, which comes into play during generation of the initial
trees as well in passes which try to optimize branchy code into
straighter code.
Thanks. I did look at BRANCH_COST, and played with the values, but it
didn't seem to have any affect. I'll take a closer look.
Look at LOGICAL_OP_NON_SHORT_CIRCUIT . By defualt, it is defined as:
(BRANCH_COST (optimize_function_for_speed_p (cfun), \
false) >= 2)
But MIPS defines it as 0.
Changing LOGICAL_OP_NON_SHORT_CIRCUIT to 0 will disable this
optimization.
LOGICAL_OP_NON_SHORT_CIRCUIT controls both places where (cond1 == 2048
|| cond2 == 8) would remove the branch.
NOTE I think MIPS defines LOGICAL_OP_NON_SHORT_CIRCUIT incorrectly but
that is a different story.
Thanks,
Andrew Pinski
I set BRANCH_COST to 1 for both predicted and non-predicted branches.
This generates the shorter sequence with a compare, but I'm concerned
that it will adversely affect code generation elsewhere. Branches are
higher cost than simple instructions.
Looking at the code generated with BRANCH_COST > 1, it doesn't do what
I indicated above. This did not reduce the number of branches.
In both cases there are two branches in this short test case. When
BRANCH_COST > 1, there are three instructions to do a compare vs one
when BRANCH_COST = 1. I'm at a loss to see where there is any benefit.
I'll take a look at the LOGICAL_OP_NON_SHORT_CIRCUIT settings and see
if this makes a difference.
Setting LOGICAL_OP_NON_SHORT_CIRCUIT to zero generates the better code
sequence. Thanks for the pointer.
I stepped through the code at the two places where
LOGICAL_OP_NON_SHORT_CIRCUIT is tested, with it set both to 0 and 1, and
it is not clear what the code is trying to do. Or what this parameter
is really supposed to mean. I'll have to go back and look again.
Setting LOGICAL_OP_NON_SHORT_CIRCUIT to 0 or 1 does not change the
number of branches. Why this would depend on BRANCH_COST is not clear,
and especially why it would depend on the costs of branches which are
predicted to fail makes no sense to me.
Is there a use case which would help this make some sense?
--
Michael Eagerea...@eagerm.com
1960 Park Blvd., Palo Alto, CA 94306
Re: Gimpel lowering question.
On 11/28/18 14:37, Andrew Pinski wrote:
On Wed, Nov 28, 2018 at 9:47 AM Michael Eager wrote:
On 11/28/18 09:10, Jeff Law wrote:
On 11/28/18 10:00 AM, Michael Eager wrote:
I have a small test case which generates poor quality code on my target.
Here is the original:
if (cond1 == 2048 || cond2 == 8)
{
x = x + y;
}
return x;
This ends up generating a series of instructions to compute a flag with
the result of the condition followed by a single compare with zero and
a jump. Better code would be two compares and two jumps.
The gimple is
_1 = cond1 == 2048;
_2 = cond2 == 8;
_3 = _1 | _2;
if (_3 != 0) goto ; else goto ;
...
so this poor code sequence is essentially a correct translation.
On MIPS, for the same test case, the gimple is different:
if (cond1 == 2048) goto ; else goto ;
:
if (cond2 == 8) goto ; else goto ;
:
...
which generates the better code sequence.
Can someone give me a pointer where to find where the lowering
pass decides to break up a condition into multiple tests? Is
there a target configuration option which I have overlooked or
maybe set incorrectly?
BRANCH_COST, which comes into play during generation of the initial
trees as well in passes which try to optimize branchy code into
straighter code.
Thanks. I did look at BRANCH_COST, and played with the values, but it
didn't seem to have any affect. I'll take a closer look.
Look at LOGICAL_OP_NON_SHORT_CIRCUIT . By defualt, it is defined as:
(BRANCH_COST (optimize_function_for_speed_p (cfun), \
false) >= 2)
But MIPS defines it as 0.
Changing LOGICAL_OP_NON_SHORT_CIRCUIT to 0 will disable this optimization.
LOGICAL_OP_NON_SHORT_CIRCUIT controls both places where (cond1 == 2048
|| cond2 == 8) would remove the branch.
NOTE I think MIPS defines LOGICAL_OP_NON_SHORT_CIRCUIT incorrectly but
that is a different story.
Thanks,
Andrew Pinski
I set BRANCH_COST to 1 for both predicted and non-predicted branches.
This generates the shorter sequence with a compare, but I'm concerned
that it will adversely affect code generation elsewhere. Branches are
higher cost than simple instructions.
Looking at the code generated with BRANCH_COST > 1, it doesn't do what
I indicated above. This did not reduce the number of branches.
In both cases there are two branches in this short test case. When
BRANCH_COST > 1, there are three instructions to do a compare vs one
when BRANCH_COST = 1. I'm at a loss to see where there is any benefit.
I'll take a look at the LOGICAL_OP_NON_SHORT_CIRCUIT settings and see
if this makes a difference.
--
Michael Eagerea...@eagerm.com
1960 Park Blvd., Palo Alto, CA 94306
Re: Gimpel lowering question.
On 11/28/18 09:10, Jeff Law wrote:
On 11/28/18 10:00 AM, Michael Eager wrote:
I have a small test case which generates poor quality code on my target.
Here is the original:
if (cond1 == 2048 || cond2 == 8)
{
x = x + y;
}
return x;
This ends up generating a series of instructions to compute a flag with
the result of the condition followed by a single compare with zero and
a jump. Better code would be two compares and two jumps.
The gimple is
_1 = cond1 == 2048;
_2 = cond2 == 8;
_3 = _1 | _2;
if (_3 != 0) goto ; else goto ;
...
so this poor code sequence is essentially a correct translation.
On MIPS, for the same test case, the gimple is different:
if (cond1 == 2048) goto ; else goto ;
:
if (cond2 == 8) goto ; else goto ;
:
...
which generates the better code sequence.
Can someone give me a pointer where to find where the lowering
pass decides to break up a condition into multiple tests? Is
there a target configuration option which I have overlooked or
maybe set incorrectly?
BRANCH_COST, which comes into play during generation of the initial
trees as well in passes which try to optimize branchy code into
straighter code.
Thanks. I did look at BRANCH_COST, and played with the values, but it
didn't seem to have any affect. I'll take a closer look.
--
Michael Eagerea...@eagerm.com
1960 Park Blvd., Palo Alto, CA 94306
Gimpel lowering question.
I have a small test case which generates poor quality code on my target.
Here is the original:
if (cond1 == 2048 || cond2 == 8)
{
x = x + y;
}
return x;
This ends up generating a series of instructions to compute a flag with
the result of the condition followed by a single compare with zero and
a jump. Better code would be two compares and two jumps.
The gimple is
_1 = cond1 == 2048;
_2 = cond2 == 8;
_3 = _1 | _2;
if (_3 != 0) goto ; else goto ;
...
so this poor code sequence is essentially a correct translation.
On MIPS, for the same test case, the gimple is different:
if (cond1 == 2048) goto ; else goto ;
:
if (cond2 == 8) goto ; else goto ;
:
...
which generates the better code sequence.
Can someone give me a pointer where to find where the lowering
pass decides to break up a condition into multiple tests? Is
there a target configuration option which I have overlooked or
maybe set incorrectly?
--
Michael Eagerea...@eagerm.com
1960 Park Blvd., Palo Alto, CA 94306
Re: Question about make_extraction() in combine.c
On 11/21/18 11:47, Segher Boessenkool wrote: On Wed, Nov 21, 2018 at 08:52:21AM -0800, Michael Eager wrote: On 11/21/2018 08:33 AM, Segher Boessenkool wrote: On Tue, Nov 20, 2018 at 10:07:35AM -0800, Michael Eager wrote: The internal RTL should not be dictating what the target arch can or cannot implement. Reload should insert any needed conversions, especially ones which narrow the size. Well, that depends. A zero_extract of mem is only defined for byte_mode, just like SET is only defined for VOIDmode (on the SET itself, not its args). Because this is guaranteed, nothing in GCC ever needs to check this. That is the theory of course; in reality quite a few targets have used other modes for the mem in a zero_extract, and this seems to have mostly worked. This restriction on zero_extract MEM args (and only MEM) seems to be completely arbitrary. What is it about the operation of extracting a bit field which makes it dependent on the memory access size? If the mode is required to be byte_mode, then all code dealing with it can assume it to be byte_mode. Without first having to check it, and without having to handle other modes. Essentially all other RTL operations place no restriction on mode. If there are no arbitrary mode restrictions, then no checking is required. The value of SET is VOIDmode, in that it has no value. Not sure what your point is here. See above. As another example, closer by, an extract length of 0 is not allowed either, for zero_extract. And this *did* cause problems recently. This is a restriction which does make sense. It isn't clear what the value of a zero length field is, or how to represent it. If something is undefined, then there is a strong argument for making is invalid. (Are there architectures which have instructions which extract a zero length bit field? I doubt it.) It doesn't matter if you (or I, or anyone) think it makes sense; the rules are the rules. If you want to change the rules, then post a patch. These are not immutable rules delivered by some deity. Rules which have no discernible value or purpose can and should be changed. A patch is being tested. Why was it documented as requiring byte mode? Was this changed, just the documentation was not updated? Ancient history. As Jeff said, perhaps an architectural requirement of VAX or m68k. This wasn't changed, as far as I'm aware. So a mem in a *_extract is still required to be byte_mode you say? > I don't think we can change this in the documentation without reviewing all code that deals with *_extract to see if this will work :-/ Have at it. -- Michael Eagerea...@eagerm.com 1960 Park Blvd., Palo Alto, CA 94306
Re: Question about make_extraction() in combine.c
On 11/21/2018 08:33 AM, Segher Boessenkool wrote: On Tue, Nov 20, 2018 at 10:07:35AM -0800, Michael Eager wrote: The internal RTL should not be dictating what the target arch can or cannot implement. Reload should insert any needed conversions, especially ones which narrow the size. Well, that depends. A zero_extract of mem is only defined for byte_mode, just like SET is only defined for VOIDmode (on the SET itself, not its args). Because this is guaranteed, nothing in GCC ever needs to check this. That is the theory of course; in reality quite a few targets have used other modes for the mem in a zero_extract, and this seems to have mostly worked. This restriction on zero_extract MEM args (and only MEM) seems to be completely arbitrary. What is it about the operation of extracting a bit field which makes it dependent on the memory access size? The value of SET is VOIDmode, in that it has no value. Not sure what your point is here. As another example, closer by, an extract length of 0 is not allowed either, for zero_extract. And this *did* cause problems recently. This is a restriction which does make sense. It isn't clear what the value of a zero length field is, or how to represent it. If something is undefined, then there is a strong argument for making is invalid. (Are there architectures which have instructions which extract a zero length bit field? I doubt it.) Why was it documented as requiring byte mode? Was this changed, just the documentation was not updated? Ancient history. As Jeff said, perhaps an architectural requirement of VAX or m68k. This wasn't changed, as far as I'm aware. -- Michael Eagerea...@eagerm.com 1960 Park Blvd., Palo Alto, CA 94306
Re: Question about make_extraction() in combine.c
On 11/18/2018 08:14 AM, Jeff Law wrote:
On 11/18/18 8:44 AM, Michael Eager wrote:
On 11/16/18 14:50, Segher Boessenkool wrote:
Hi!
On Wed, Nov 14, 2018 at 11:22:58AM -0800, Michael Eager wrote:
The (mem:SI) is converted to (mem:QI).
The return from make_extract() is
(zero_extract:SI (mem:QI (reg/v/f:SI 50 [ gp ]))
(const_int 1 [0x1])
(const_int 2 [0x2]))
The target has an instruction pattern for zero_extract, but it matches
SI values, not QI. So the instruction which implements a test of a bit
flag is never generated.
The RTL documentation says:
If @var{loc} is in memory, its mode must be a single-byte integer
mode.
But obviously various ports use other modes, too. I wonder if that ever
worked well.
Thanks. I hadn't noticed this.
Does anyone have any idea why there is a restriction on the mode?
Other instruction patterns don't place arbitrary restriction on the
memory access mode.
Not offhand. BUt it's been around for a long time (at least since the
early 90s). I stumbled over it several times through the years. It
could well be an artifact of the m68k or the vax.
The internal RTL should not be dictating what the target arch can or
cannot implement. Reload should insert any needed conversions,
especially ones which narrow the size.
I have a patch which removes this conversion. Works for my target. I
built and ran 'make check' for x86 with no additional failures. I don't
have a VAX or M68K sitting around to test. :-)
I can submit the patch and remove the restriction from the docs, but I
have no way of telling that this doesn't break (or deoptimize) some
other target.
--
Michael Eagerea...@eagerm.com
1960 Park Blvd., Palo Alto, CA 94306
Question about make_extraction() in combine.c
In combine, simplify_comparison() is being called with the following arguments: code = EQ op0 = (and:SI (mem:SI (reg/v/f:SI 50 [ gp ]) (const_int 4 [0x4])) op1 = (const_int 0 [0]) After churning down through make_compound_operation() and make_compound_operation_int(), processing gets to make_extraction(). Eventually (combine.c:7753), make_extract() decides that the best pattern is EP_epextzv. No instruction patterns are provided for "extz*"; get_best_reg_extraction_insn() returns false, and make_extraction falls through to this code [indenting modified for email]. combine.c:7786: /* Be careful not to go beyond the extracted object and maintain the natural alignment of the memory. */ wanted_inner_mode = smallest_int_mode_for_size (len); while (pos % GET_MODE_BITSIZE (wanted_inner_mode) + len > GET_MODE_BITSIZE (wanted_inner_mode)) wanted_inner_mode = GET_MODE_WIDER_MODE (wanted_inner_mode).require (); Len == 1; the result is that wanted_inner_mode == E_QImode. No instruction patterns are provided for "extz*"; get_best_reg_extraction_insn() returns false, and make_extraction falls through to this code The (mem:SI) is converted to (mem:QI). The return from make_extract() is (zero_extract:SI (mem:QI (reg/v/f:SI 50 [ gp ])) (const_int 1 [0x1]) (const_int 2 [0x2])) The target has an instruction pattern for zero_extract, but it matches SI values, not QI. So the instruction which implements a test of a bit flag is never generated. In an old version of GCC, a call to mode_dependent_address_p() controlled whether this conversion from SI to QI was done. This would not be useful, since QI is a valid address mode in general, just not for the zero_extract pattern. A kludge was added to prevent this conversion from SI to QI. I'd like to avoid re-implementing the kludge. Questions: 1. What is make_extract() trying to do with the MEM address? 2. Why modify the MEM address from SI to QI? There's no obvious benefit that I see of (zero_extract:SI (mem:QI)...) over (zero_extract:SI (mem:SI)...). 3. What's the best way to fix this? - Remove the down-sizing of MEM in make_extract()? - Define patterns for extz*? - Do something so zero_extend accepts QI? -- Michael Eagerea...@eagerm.com 1960 Park Blvd., Palo Alto, CA 94306
Re: [PATCH] [Microblaze]: PIC Data Text Relative
Committed.
On 05/05/2018 04:14 AM, Andrew Sadek wrote:
Hello Michael,
I made a re-run for the GCC test suite.
I have just noticed that picdtr.c was not uploaded, also I forgot to
adapt the new pic option text in it.
thus we need to apply the attached patch.
Otherwise, it's all fine.
Results (Normal Run):
=== gcc Summary ===
# of expected passes 91211
# of unexpected failures 1325
# of unexpected successes 3
# of expected failures 212
# of unresolved testcases 374
# of unsupported tests 2863
Results (with -fPIE -mpic-data-is-text-relative):
=== gcc Summary ===
# of expected passes 91243
# of unexpected failures 1208
# of unexpected successes 3
# of expected failures 212
# of unresolved testcases 374
# of unsupported tests 2888
Comparison for PASS-> FAIL is only this one now:
PASS->FAIL: gcc.dg/uninit-19.c (test for excess errors) => already fails
with -fPIE/-fPIC as mentioned before.
On Thu, May 3, 2018 at 7:13 PM, Andrew Sadek <andrew.sadek...@gmail.com
<mailto:andrew.sadek...@gmail.com>> wrote:
--resend Michael's reply
On Mon, Apr 30, 2018 at 1:19 PM, Michael Eager <ea...@eagerm.com
<mailto:ea...@eagerm.com>> wrote:
Applied -- Committed revision 259758.
Andrew -- Please re-run GCC regression test to verify that nothing
was lost in the editing.
--
Michael Eager ea...@eagerm.com <mailto:ea...@eagerm.com>
1960 Park Blvd., Palo Alto, CA 94306
--
Andrew
--
Andrew
--
Michael Eagerea...@eagerm.com
1960 Park Blvd., Palo Alto, CA 94306
Re: [PATCH] [Microblaze]: PIC Data Text Relative
On 04/19/2018 03:43 AM, Andrew Sadek wrote:
On Wed, Apr 18, 2018 at 6:57 PM, Michael Eager <ea...@eagercon.com> wrote:
Hi Andrew --
Check indents in the following files:
(Make sure that your tabs are set at 8 chars.)
--- gcc/config/microblaze/microblaze.c
--- gcc/config/microblaze/microblaze.md
I have re-run check_GNU_Style.sh and no are issues are found now.
I corrected a large number of indent problems in microblaze.c.
Just a couple coding notes:
microblaze.c:
@@ -858,6 +879,16 @@ microblaze_classify_address (struct microblaze_add
+ && strict == 2)
Include comment in function header describing meaning of strict == 2.
Done
@@ -1022,7 +1065,7 @@ microblaze_legitimize_address (rtx x, rtx oldx ATT
else if (flag_pic == 2 && !TARGET_PIC_DATA_TEXT_REL)
{
...
}
else if (flag_pic == 2 && TARGET_PIC_DATA_TEXT_REL)
{
...
}
It's better to factor this into
else if (flag_pic == 2)
{
if (TARGET_PIC_DATA_TEXT_REL)
{
...
}
else
{
...
}
}
Done
This code pattern appears twice in microblaze_legitimize_address.
I corrected the second one.
The code in the second occurrence can be refactored to move
if (reload_in_progress)
df_set_regs_ever_live (PIC_OFFSET_TABLE_REGNUM, true);
out of the if(TARGET_PIC_DATA_TEXT_REL) brackets. (Done.)
Please send me an updated ChangeLog, including testsuite.
--
Michael Eagerea...@eagerm.com
1960 Park Blvd., Palo Alto, CA 94306
Re: [PATCH] [Microblaze]: PIC Data Text Relative
ctor this into
else if (flag_pic == 2)
{
if (TARGET_PIC_DATA_TEXT_REL)
{
...
}
else
{
...
}
}
microblaze.md:
@@ -1848,7 +1850,7 @@
+if (!flag_pic || (flag_pic && TARGET_PIC_DATA_TEXT_REL))
Test for flag_pic is redundant. This can be
+if (!flag_pic || TARGET_PIC_DATA_TEXT_REL)
doc/invoke.texi:
-mpic-data-text-rel -- include description
Is this different from -mpic-data-is-text-relative?
(Yes, that's a bit of a wordy option.)
doc/tm.texi:
+Return a flag for either generating ADDR_DIF_VEC table
+or ADDR_VEC table for jumps in case of -fPIC.
Explicitly state what true or false means.
target.def:
+(generate_pic_addr_diff_vec,
Explicitly state what true or false means.
targhooks.c:
+bool
+default_generate_pic_addr_diff_vec (void)
+{
+ return true;
+}
This doesn't appear to match the description in target.def.
--
Michael Eagerea...@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306
Re: [PATCH] [Microblaze]: PIC Data Text Relative
Hi Andrew --
I just do a visual check. The script can help.
On 03/20/2018 07:13 AM, Andrew Sadek wrote:
Many Thanks Michael for the updates .. I was actually looking for something
similar for my test case .. I m currently revising the Gnu code conventions
on the patches then will send them again.
I m actually running the 'check_GNU_Style' in contrib folder,, is this the
correct way ? Is it enough ?
Andrew
On Tue, Mar 20, 2018, 03:30 Michael Eager <ea...@eagerm.com> wrote:
Also check the { dg-skip-if } directive.
https://gcc.gnu.org/onlinedocs/gccint/Directives.html
On 03/19/2018 06:14 PM, Michael Eager wrote:
Hi Andrew --
Please take a look at the test case description:
https://gcc.gnu.org/wiki/HowToPrepareATestcase
and see if you can do one of the following:
- Modify the regex expression in the scan-assembler to accept
either format of generated output
or
- Add { dg-option } directives to turn off your new options
if specified. (You should be able to specify -mno-pic)
or
- Duplicate the test cases and add { dg-option } directives
to specify the correct options, with and without your
new options.
or
- Add test cases with a { dg-option } directive with your
new options.
This is not required -- your patch appears to work OK. Normally,
the new PIC Data options would not be used when running the test
suite, so the tests would not fail. It's just nice to have the
test suite updated when new options are added.
On 03/19/2018 01:07 PM, Michael Eager wrote:
Hi Andrew --
Good work.
Please submit your updated patch. Check that you follow
GNU coding standards. Also make sure that the new options
are documented in gcc/doc/invoke.texi.
On 03/18/18 03:27, Andrew Sadek wrote:
Hello Michael,
I have run the test using the new PIC options.
Actually, I have discovered 2 unhandled cases in
'microblaze_expand_move' + missing conditions in linker relax
leading some test cases execution to fail.
After fixing them, I made a re-run for the whole regression, and the
results analogy below:
Original, without my patch:
=== gcc Summary ===
# of expected passes90776
# of unexpected failures1317
# of unexpected successes3
# of expected failures207
# of unresolved testcases115
# of unsupported tests2828
With my patch, calling '-fPIE - mpic-data-text-rel'
=== gcc Summary ===
# of expected passes90843
# of unexpected failures1256
# of unexpected successes3
# of expected failures207
# of unresolved testcases115
# of unsupported tests2853
After running the 'dg-cmp-results.sh' in contrib folder, the
PASS->FAIL are below:
PASS->FAIL: gcc.dg/uninit-19.c (test for excess errors)
PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O0
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O1
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O2 -flto
-fno-use-linker-plugin -flto-partition=none scan-assembler
lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects scan-assembler
lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O2
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O3 -g
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var1.c -Os
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O0
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O1
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O2 -flto
-fno-use-linker-plugin -flto-partition=none scan-assembler
lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects scan-assembler
lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O2
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O3 -g
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var2.c -Os
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/sdata_var1.c -O0
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13
PASS->FAIL: gcc.target/microblaze/others/sdata_var1.c -O1
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13
PASS->FAIL: gcc.target/microblaze/others/sdata_var1.c -O2 -flto
-fno-use-linker-plugin -flto-partition=none scan-assembler
lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13
PASS->FAIL: gcc.target/microblaze/others/sdata_
Re: [PATCH] [Microblaze]: PIC Data Text Relative
Also check the { dg-skip-if } directive.
https://gcc.gnu.org/onlinedocs/gccint/Directives.html
On 03/19/2018 06:14 PM, Michael Eager wrote:
Hi Andrew --
Please take a look at the test case description:
https://gcc.gnu.org/wiki/HowToPrepareATestcase
and see if you can do one of the following:
- Modify the regex expression in the scan-assembler to accept
either format of generated output
or
- Add { dg-option } directives to turn off your new options
if specified. (You should be able to specify -mno-pic)
or
- Duplicate the test cases and add { dg-option } directives
to specify the correct options, with and without your
new options.
or
- Add test cases with a { dg-option } directive with your
new options.
This is not required -- your patch appears to work OK. Normally,
the new PIC Data options would not be used when running the test
suite, so the tests would not fail. It's just nice to have the
test suite updated when new options are added.
On 03/19/2018 01:07 PM, Michael Eager wrote:
Hi Andrew --
Good work.
Please submit your updated patch. Check that you follow
GNU coding standards. Also make sure that the new options
are documented in gcc/doc/invoke.texi.
On 03/18/18 03:27, Andrew Sadek wrote:
Hello Michael,
I have run the test using the new PIC options.
Actually, I have discovered 2 unhandled cases in
'microblaze_expand_move' + missing conditions in linker relax
leading some test cases execution to fail.
After fixing them, I made a re-run for the whole regression, and the
results analogy below:
Original, without my patch:
=== gcc Summary ===
# of expected passes 90776
# of unexpected failures 1317
# of unexpected successes 3
# of expected failures 207
# of unresolved testcases 115
# of unsupported tests 2828
With my patch, calling '-fPIE - mpic-data-text-rel'
=== gcc Summary ===
# of expected passes 90843
# of unexpected failures 1256
# of unexpected successes 3
# of expected failures 207
# of unresolved testcases 115
# of unsupported tests 2853
After running the 'dg-cmp-results.sh' in contrib folder, the
PASS->FAIL are below:
PASS->FAIL: gcc.dg/uninit-19.c (test for excess errors)
PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O0
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O1
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O2 -flto
-fno-use-linker-plugin -flto-partition=none scan-assembler
lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects scan-assembler
lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O2
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O3 -g
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var1.c -Os
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O0
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O1
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O2 -flto
-fno-use-linker-plugin -flto-partition=none scan-assembler
lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects scan-assembler
lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O2
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O3 -g
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var2.c -Os
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/sdata_var1.c -O0
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13
PASS->FAIL: gcc.target/microblaze/others/sdata_var1.c -O1
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13
PASS->FAIL: gcc.target/microblaze/others/sdata_var1.c -O2 -flto
-fno-use-linker-plugin -flto-partition=none scan-assembler
lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13
PASS->FAIL: gcc.target/microblaze/others/sdata_var1.c -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects scan-assembler
lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13
PASS->FAIL: gcc.target/microblaze/others/sdata_var1.c -O2
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13
PASS->FAIL: gcc.target/microblaze/others/sdata_var1.c -O3 -g
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13
PASS->FAIL: gcc.target/microblaze/others/sdata_var1.c -Os
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13
PASS->FAIL: gcc
Re: [PATCH] [Microblaze]: PIC Data Text Relative
Hi Andrew --
Please take a look at the test case description:
https://gcc.gnu.org/wiki/HowToPrepareATestcase
and see if you can do one of the following:
- Modify the regex expression in the scan-assembler to accept
either format of generated output
or
- Add { dg-option } directives to turn off your new options
if specified. (You should be able to specify -mno-pic)
or
- Duplicate the test cases and add { dg-option } directives
to specify the correct options, with and without your
new options.
or
- Add test cases with a { dg-option } directive with your
new options.
This is not required -- your patch appears to work OK. Normally,
the new PIC Data options would not be used when running the test
suite, so the tests would not fail. It's just nice to have the
test suite updated when new options are added.
On 03/19/2018 01:07 PM, Michael Eager wrote:
Hi Andrew --
Good work.
Please submit your updated patch. Check that you follow
GNU coding standards. Also make sure that the new options
are documented in gcc/doc/invoke.texi.
On 03/18/18 03:27, Andrew Sadek wrote:
Hello Michael,
I have run the test using the new PIC options.
Actually, I have discovered 2 unhandled cases in
'microblaze_expand_move' + missing conditions in linker relax
leading some test cases execution to fail.
After fixing them, I made a re-run for the whole regression, and the
results analogy below:
Original, without my patch:
=== gcc Summary ===
# of expected passes 90776
# of unexpected failures 1317
# of unexpected successes 3
# of expected failures 207
# of unresolved testcases 115
# of unsupported tests 2828
With my patch, calling '-fPIE - mpic-data-text-rel'
=== gcc Summary ===
# of expected passes 90843
# of unexpected failures 1256
# of unexpected successes 3
# of expected failures 207
# of unresolved testcases 115
# of unsupported tests 2853
After running the 'dg-cmp-results.sh' in contrib folder, the
PASS->FAIL are below:
PASS->FAIL: gcc.dg/uninit-19.c (test for excess errors)
PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O0
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O1
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O2 -flto
-fno-use-linker-plugin -flto-partition=none scan-assembler
lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects scan-assembler
lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O2
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var1.c -O3 -g
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var1.c -Os
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O0
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O1
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O2 -flto
-fno-use-linker-plugin -flto-partition=none scan-assembler
lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects scan-assembler
lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O2
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var2.c -O3 -g
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/data_var2.c -Os
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r0
PASS->FAIL: gcc.target/microblaze/others/sdata_var1.c -O0
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13
PASS->FAIL: gcc.target/microblaze/others/sdata_var1.c -O1
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13
PASS->FAIL: gcc.target/microblaze/others/sdata_var1.c -O2 -flto
-fno-use-linker-plugin -flto-partition=none scan-assembler
lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13
PASS->FAIL: gcc.target/microblaze/others/sdata_var1.c -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects scan-assembler
lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13
PASS->FAIL: gcc.target/microblaze/others/sdata_var1.c -O2
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13
PASS->FAIL: gcc.target/microblaze/others/sdata_var1.c -O3 -g
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13
PASS->FAIL: gcc.target/microblaze/others/sdata_var1.c -Os
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13
PASS->FAIL: gcc.target/microblaze/others/sdata_var2.c -O0
scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13
PASS->FAIL: gcc.target/microblaze/others/sdata_
Re: [PATCH] [Microblaze]: PIC Data Text Relative
g scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13 PASS->FAIL: gcc.target/microblaze/others/sdata_var4.c -Os scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13 PASS->FAIL: gcc.target/microblaze/others/sdata_var5.c -O0 scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13 PASS->FAIL: gcc.target/microblaze/others/sdata_var5.c -O1 scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13 PASS->FAIL: gcc.target/microblaze/others/sdata_var5.c -O2 -flto -fno-use-linker-plugin -flto-partition=none scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13 PASS->FAIL: gcc.target/microblaze/others/sdata_var5.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13 PASS->FAIL: gcc.target/microblaze/others/sdata_var5.c -O2 scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13 PASS->FAIL: gcc.target/microblaze/others/sdata_var5.c -O3 -g scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13 PASS->FAIL: gcc.target/microblaze/others/sdata_var5.c -Os scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13 PASS->FAIL: gcc.target/microblaze/others/string_cst1_gpopt.c -O0 scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13 PASS->FAIL: gcc.target/microblaze/others/string_cst1_gpopt.c -O1 scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13 PASS->FAIL: gcc.target/microblaze/others/string_cst1_gpopt.c -O2 -flto -fno-use-linker-plugin -flto-partition=none scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13 PASS->FAIL: gcc.target/microblaze/others/string_cst1_gpopt.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13 PASS->FAIL: gcc.target/microblaze/others/string_cst1_gpopt.c -O2 scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13 PASS->FAIL: gcc.target/microblaze/others/string_cst1_gpopt.c -O3 -g scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13 PASS->FAIL: gcc.target/microblaze/others/string_cst1_gpopt.c -Os scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13 PASS->FAIL: gcc.target/microblaze/others/string_cst2_gpopt.c -O0 scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13 PASS->FAIL: gcc.target/microblaze/others/string_cst2_gpopt.c -O1 scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13 PASS->FAIL: gcc.target/microblaze/others/string_cst2_gpopt.c -O2 -flto -fno-use-linker-plugin -flto-partition=none scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13 PASS->FAIL: gcc.target/microblaze/others/string_cst2_gpopt.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13 PASS->FAIL: gcc.target/microblaze/others/string_cst2_gpopt.c -O2 scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13 PASS->FAIL: gcc.target/microblaze/others/string_cst2_gpopt.c -O3 -g scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13 PASS->FAIL: gcc.target/microblaze/others/string_cst2_gpopt.c -Os scan-assembler lwi\tr([0-9]|[1-2][0-9]|3[0-1]),r13 the 'gcc.dg/uninit-19.c ' already fails with -fPIE, and for the others in microblaze, expectation is r0 or r13 as a base register which is not the case here 'r20', and these tests also fails with -fPIE. Moreover, the test cases that failed due 'unresolved GLOBAL_OFFSET_TABLE' now pass as we do not have GOT in data text relative. Attached the compare output of the scripit with verbosity 2. Please tell me if any adjustments needed or If I need to re-run nything. Thanks On Tue, Mar 13, 2018 at 10:44 PM, Michael Eager <ea...@eagerm.com <mailto:ea...@eagerm.com>> wrote: On 03/13/18 00:56, Andrew Sadek wrote: Ok, so you mean with '-fPIC -mpic-data-text-relative' as I do in my test case ? If all is Ok, execution and compilation shall ideally pass for the test cases. Correct. I want to make sure of two things: -- Your patch doesn't break anything (i.e., cause a regression) when you don't use the options. This seems complete. -- Your patch works as intended when you do use the options. But I have noticed that there are some output pattern checks done in some test cases and this may fail since the output assembly is different, anyway I shall give it a try and send you the results with the new options. There should be no target dependencies in the generic GCC tests. Different instruction patterns which generate the correct results should not be a problem. -- Michael Eager ea...@eagerm.com <mailto:ea...@eagerm.com> 1960 Park Blvd., Palo Alto, CA 94306 -- Andrew -- Andrew -- Michael Eagerea...@eagerm.com 1960 Park Blvd., Palo Alto, CA 94306
Re: [PATCH] [Microblaze]: PIC Data Text Relative
On 03/13/18 00:56, Andrew Sadek wrote: Ok, so you mean with '-fPIC -mpic-data-text-relative' as I do in my test case ? If all is Ok, execution and compilation shall ideally pass for the test cases. Correct. I want to make sure of two things: -- Your patch doesn't break anything (i.e., cause a regression) when you don't use the options. This seems complete. -- Your patch works as intended when you do use the options. But I have noticed that there are some output pattern checks done in some test cases and this may fail since the output assembly is different, anyway I shall give it a try and send you the results with the new options. There should be no target dependencies in the generic GCC tests. Different instruction patterns which generate the correct results should not be a problem. -- Michael Eagerea...@eagerm.com 1960 Park Blvd., Palo Alto, CA 94306
Re: [PATCH] [Microblaze]: PIC Data Text Relative
On 03/12/18 23:10, Andrew Sadek wrote:
_Command for running testsuite:_
/make -k check-gcc RUNTESTFLAGS="-v --target_board=microblaze-qemu
CFLAGS_FOR_TARGET='-include /home/andrew/qemu/common.h
-L/home/andrew/qemu/lib -Wl,--start-group,-lxil,-lgcc,-lc,--end-group
-mlittle-endian' "/
_Quick Details:_
1) common.h: Here I define 'STACK_SIZE' with (0x4000) as it's used in
some test cases.
2) -L . /lib: it contains libm, libc in little endian since we use
'qemu-system-microblazeel', and libxil which is the libgloss + some
additional features (read, write, inbyte, outbyte) built with Xilinx SDK.
_mb-gcc command example from gcc.log:_
Testing execute/va-arg-15.c, -O1
doing compile
Executing on
host://home/andrew/gcc_workspace/gcc_orig/microblaze-xilinx-elf/build/build-cc-gcc-final/gcc/xgcc
-B/home/andrew/gcc_workspace/gcc_orig/microblaze-xilinx-elf/build/build-cc-gcc-final/gcc/
/home/andrew/gcc_workspace/gcc_orig/microblaze-xilinx-elf/src/gcc/gcc/testsuite/gcc.c-torture/execute/va-arg-15.c
-include /home/andrew/qemu/common.h -L/home/andrew/qemu/lib
-Wl,--start-group,-lxil,-lgcc,-lc,--end-group -mlittle-endian
-fno-diagnostics-show-caret -fdiagnostics-color=never -O1 -w
-T/home/andrew/qemu/qemu/microblazeel-softmmu/xilinx.ld -lm -o
./va-arg-15.exe (timeout = 300)/
OK. This shows that the patch does not cause regressions when the new
options are not used. That is good.
Please run the same regression test with the new PIC options. Ideally
you should have the same results.
On Mon, Mar 12, 2018 at 4:30 PM, Michael Eager <ea...@eagerm.com
<mailto:ea...@eagerm.com>> wrote:
On 03/12/18 06:19, Andrew Sadek wrote:
On Mon, Mar 5, 2018 at 9:21 PM, Michael Eager <ea...@eagerm.com
<mailto:ea...@eagerm.com> <mailto:ea...@eagerm.com
<mailto:ea...@eagerm.com>>> wrote:
On 03/02/2018 08:12 AM, Andrew Sadek wrote:
Hello Michael,
I tried running the whole GCC test suite on the current
head
(without my patch) along with 'microblaze-qemu' but I
have the
following problems:
1) The test is hanging at
'gcc.c-torture/string-large-1.c' , the
gcc is making a 100% CPU usage and the machine stucks.
I tried compiling the file alone, it generated a couple of
warnings than it hangs.
warning: '__builtin_memchr' specified size
4294967295 exceeds
maximum object size 2147483647 [-Wstringop-overflow=]
vp1 = __builtin_memchr (a, b, SIZE1);
Is it a bug? Is there something wrong with my
configuration ?
GCC configured with options : --with-newlib
--enable-threads=no
--disable-shared --with-pkgversion='crosstool-NG
crosstool-ng-1.23.0-280-g01e3290' --enable-__cxa_atexit
--disable-libgomp --disable-libmudflap --disable-libmpx
--disable-libssp --disable-libquadmath
--disable-libquadmath-support --enable-lto
--with-host-libstdcxx='-static-libgcc
-Wl,-Bstatic,-lstdc++,-Bdynamic -lm'
--enable-target-optspace
--disable-nls --enable-multiarch --enable-languages=c,c++
Your configuration is more complex than my hard-metal
target version,
but it looks reasonable.
The problem with string-large-1.c does appear to be a bug.
You can
add a line to the test case which will mark it as known
failure for MB:
/* { dg-xfail-if "exceeds maximum" { microblaze-*-* } } */
(I have not tested this, but it should work. Compare with
other
xfail's.)
Problem that the whole compile package is invoked with '-w'
which inhibits all warnings and overrides ' -Werror' as well as
' -Wfatal-errors' .
As a result, the warning message does not appear when running
compile.exp. Any suggestions ? Do I remove the '-w' ?
2) For running QEMU, I have no problem with elf
execution. But I
do not know how to auto terminate the QEMU itself as
it remains
up even after program execution.
Is there some command to be passed to QEMU in order
make system
shut down after program termination with its exit code ?
Yes, this is a problem. For other targets using QEMU I
have added dummy
HLT instructions to terminate QEMU, or used a wrapper
around QEMU which
sets breakpoints at exit (or _exit) and stops the simulator
when hit.
If you are running Linux on QEMU, instead of using QEMU
Re: [PATCH] [Microblaze]: PIC Data Text Relative
On 03/12/18 06:19, Andrew Sadek wrote:
On Mon, Mar 5, 2018 at 9:21 PM, Michael Eager <ea...@eagerm.com
<mailto:ea...@eagerm.com>> wrote:
On 03/02/2018 08:12 AM, Andrew Sadek wrote:
Hello Michael,
I tried running the whole GCC test suite on the current head
(without my patch) along with 'microblaze-qemu' but I have the
following problems:
1) The test is hanging at 'gcc.c-torture/string-large-1.c' , the
gcc is making a 100% CPU usage and the machine stucks.
I tried compiling the file alone, it generated a couple of
warnings than it hangs.
warning: '__builtin_memchr' specified size 4294967295 exceeds
maximum object size 2147483647 [-Wstringop-overflow=]
vp1 = __builtin_memchr (a, b, SIZE1);
Is it a bug? Is there something wrong with my configuration ?
GCC configured with options : --with-newlib --enable-threads=no
--disable-shared --with-pkgversion='crosstool-NG
crosstool-ng-1.23.0-280-g01e3290' --enable-__cxa_atexit
--disable-libgomp --disable-libmudflap --disable-libmpx
--disable-libssp --disable-libquadmath
--disable-libquadmath-support --enable-lto
--with-host-libstdcxx='-static-libgcc
-Wl,-Bstatic,-lstdc++,-Bdynamic -lm' --enable-target-optspace
--disable-nls --enable-multiarch --enable-languages=c,c++
Your configuration is more complex than my hard-metal target version,
but it looks reasonable.
The problem with string-large-1.c does appear to be a bug. You can
add a line to the test case which will mark it as known failure for MB:
/* { dg-xfail-if "exceeds maximum" { microblaze-*-* } } */
(I have not tested this, but it should work. Compare with other
xfail's.)
Problem that the whole compile package is invoked with '-w' which
inhibits all warnings and overrides ' -Werror' as well as '
-Wfatal-errors' .
As a result, the warning message does not appear when running
compile.exp. Any suggestions ? Do I remove the '-w' ?
2) For running QEMU, I have no problem with elf execution. But I
do not know how to auto terminate the QEMU itself as it remains
up even after program execution.
Is there some command to be passed to QEMU in order make system
shut down after program termination with its exit code ?
Yes, this is a problem. For other targets using QEMU I have added dummy
HLT instructions to terminate QEMU, or used a wrapper around QEMU which
sets breakpoints at exit (or _exit) and stops the simulator when hit.
If you are running Linux on QEMU, instead of using QEMU's built-in gdb
interface you might use the Linux system as the target for the test
suite, running gdbserver on the target.
I have finally managed to fully run QEMU with test suite but had to make
a local modification in the QEMU code itself.
In the translate function, I make a check if "bri 0" is reached which is
the '_exit'. Then, abort the QEMU app with the exit code stored in 'r5'.
I've done essentially the same for other targets.
Here are the results below:
_Without Patch:_
=== gcc Summary ===
# of expected passes90776
# of unexpected failures 1317
# of unexpected successes3
# of expected failures207
# of unresolved testcases115
# of unsupported tests2828
_With Patch and after adding my test case:_
=== gcc Summary ===
# of expected passes90790
# of unexpected failures1317
# of unexpected successes3
# of expected failures207
# of unresolved testcases115
# of unsupported tests2828
This appears to be reasonable results. It appears that there are no
regressions.
Please send me the mb-gcc command line options for both of these test runs.
--
Michael Eagerea...@eagerm.com
1960 Park Blvd., Palo Alto, CA 94306
Re: [PATCH] [Microblaze]: PIC Data Text Relative
On 03/02/2018 08:12 AM, Andrew Sadek wrote:
Hello Michael,
I tried running the whole GCC test suite on the current head (without my
patch) along with 'microblaze-qemu' but I have the following problems:
1) The test is hanging at 'gcc.c-torture/string-large-1.c' , the gcc is
making a 100% CPU usage and the machine stucks.
I tried compiling the file alone, it generated a couple of warnings than
it hangs.
warning: '__builtin_memchr' specified size 4294967295 exceeds maximum
object size 2147483647 [-Wstringop-overflow=]
vp1 = __builtin_memchr (a, b, SIZE1);
Is it a bug? Is there something wrong with my configuration ?
GCC configured with options : --with-newlib --enable-threads=no
--disable-shared --with-pkgversion='crosstool-NG
crosstool-ng-1.23.0-280-g01e3290' --enable-__cxa_atexit
--disable-libgomp --disable-libmudflap --disable-libmpx --disable-libssp
--disable-libquadmath --disable-libquadmath-support --enable-lto
--with-host-libstdcxx='-static-libgcc -Wl,-Bstatic,-lstdc++,-Bdynamic
-lm' --enable-target-optspace --disable-nls --enable-multiarch
--enable-languages=c,c++
Your configuration is more complex than my hard-metal target version,
but it looks reasonable.
The problem with string-large-1.c does appear to be a bug. You can
add a line to the test case which will mark it as known failure for MB:
/* { dg-xfail-if "exceeds maximum" { microblaze-*-* } } */
(I have not tested this, but it should work. Compare with other
xfail's.)
2) For running QEMU, I have no problem with elf execution. But I do not
know how to auto terminate the QEMU itself as it remains up even after
program execution.
Is there some command to be passed to QEMU in order make system shut
down after program termination with its exit code ?
Yes, this is a problem. For other targets using QEMU I have added dummy
HLT instructions to terminate QEMU, or used a wrapper around QEMU which
sets breakpoints at exit (or _exit) and stops the simulator when hit.
If you are running Linux on QEMU, instead of using QEMU's built-in gdb
interface you might use the Linux system as the target for the test
suite, running gdbserver on the target.
On Tue, Feb 27, 2018 at 10:13 AM, Andrew Sadek
<andrew.sadek...@gmail.com <mailto:andrew.sadek...@gmail.com>> wrote:
Thanks Micheal for your response.
I shall re-submit patches separately after re-running the whole GCC
Test suite and re-checking code conventions.
For sending to gdb-patches, it was a conflict from my side as
actually I thought it is also for binutils.
On Tue, Feb 27, 2018 at 2:07 AM, Michael Eager <ea...@eagerm.com
<mailto:ea...@eagerm.com>> wrote:
On 02/25/2018 11:44 PM, Andrew Guirguis wrote:
Dears,
Kindly find attached the patch bundle for Microblaze
'-mpic-data-text-relative' feature.
Description of the feature in the following link:
https://github.com/andrewsadek/microblaze-pic-data-text-rel/blob/pic_data_text_rel/README.md
<https://github.com/andrewsadek/microblaze-pic-data-text-rel/blob/pic_data_text_rel/README.md>
<https://github.com/andrewsadek/microblaze-pic-data-text-rel/blob/pic_data_text_rel/README.md
<https://github.com/andrewsadek/microblaze-pic-data-text-rel/blob/pic_data_text_rel/README.md>>
Bundle includes:
1) Change logs for GCC, binutils
2) GCC Test results and comparison with the original.
3) New Test case (picdtr.c)
4) The Patches (against current heads)
Hi Andrew --
Thanks for the submission. I have the following recommendations:
Submit each patch to the appropriate project mailing list. Only
submit
the patch for the specific project, without patches for other
projects.
Include a description of the changes with each patch as well as the
changelog. Include the patch in your email or as an attachment.
It isn't clear why you sent your submission to the gdb-patches
mailing
list, since there don't appear to be any GDB changes.
Conversely, it is
not clear why you did not include the binutils mailing list,
since you
include a patch to that project.
Be sure to follow GNU coding conventions, Check brace placement,
indent, maximum line length, if statements, etc. I noticed a number
of places where these conventions are not followed in your patches.
GCC regression tests should include all tests (e.g., gcc.dg),
not just the limited number of MicroBlaze-specific tests.
--
Michael Eager ea...@eagerm.com <mailto:ea...@eagerm.com>
1960 Park Blvd., Palo Alto, CA 94306
--
Andrew
--
Andrew
--
Michael Eagerea...@eagerm.com
1960 Park Blvd., Palo Alto, CA 94306
Re: [PATCH] [Microblaze]: PIC Data Text Relative
On 02/25/2018 11:44 PM, Andrew Guirguis wrote: Dears, Kindly find attached the patch bundle for Microblaze '-mpic-data-text-relative' feature. Description of the feature in the following link: https://github.com/andrewsadek/microblaze-pic-data-text-rel/blob/pic_data_text_rel/README.md <https://github.com/andrewsadek/microblaze-pic-data-text-rel/blob/pic_data_text_rel/README.md> Bundle includes: 1) Change logs for GCC, binutils 2) GCC Test results and comparison with the original. 3) New Test case (picdtr.c) 4) The Patches (against current heads) Hi Andrew -- Thanks for the submission. I have the following recommendations: Submit each patch to the appropriate project mailing list. Only submit the patch for the specific project, without patches for other projects. Include a description of the changes with each patch as well as the changelog. Include the patch in your email or as an attachment. It isn't clear why you sent your submission to the gdb-patches mailing list, since there don't appear to be any GDB changes. Conversely, it is not clear why you did not include the binutils mailing list, since you include a patch to that project. Be sure to follow GNU coding conventions, Check brace placement, indent, maximum line length, if statements, etc. I noticed a number of places where these conventions are not followed in your patches. GCC regression tests should include all tests (e.g., gcc.dg), not just the limited number of MicroBlaze-specific tests. -- Michael Eagerea...@eagerm.com 1960 Park Blvd., Palo Alto, CA 94306
Re: [PATCH] MicroBlaze use default ident output generation
On 11/15/2017 10:58 PM, Nathan Rossi wrote: Remove the MicroBlaze specific TARGET_ASM_OUTPUT_IDENT definition, and use the default. This resolves issues associated with the use of the .sdata2 operation in cases where emitted assembly after the ident output is incorrectly in the .sdata2 section instead of .text or any other expected section. Which results in assembly failures including operations with symbols across different segments. Can you give me an example where this causes a problem? -- Michael Eagerea...@eagerm.com 1960 Park Blvd., Palo Alto, CA 94306
Re: Enable no-exec stacks for more targets using the Linux kernel
On 09/19/2017 12:17 AM, Andreas Schwab wrote: On Sep 18 2017, Joseph Myers <jos...@codesourcery.com> wrote: Building glibc for many different configurations and running the compilation parts of the testsuite runs into failures of the elf/check-execstack test for hppa, ia64 and microblaze. ia64 is non-execstack by default, so it doesn't need any marking. The same is true for every architecture that doesn't override elf_read_implies_exec, which includes microblaze and hppa. This fails because those configurations are not generating .note.GNU-stack sections to indicate that programs do not need an executable stack. This needs to be fixed in glibc. The requirement that a null .note.GNU-stack section needs to be defined to indicate that the default stack (i.e., non-executable) is used seems backward. I don't have any problem approving the MicroBlaze GCC changes, but, like Andreas, I think that this is a glibc problem. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: regression for microblaze architecture
From https://gcc.gnu.org/ml/gcc/2017-05/msg00221.html: On 05/27/2017 09:09 AM, Michael Eager wrote: On 05/27/2017 01:51 AM, Waldemar Brodkorb wrote: Hi, Buildroot and OpenADK have samples to create a Linux system to be bootup in Qemu system emulation for microblaze architecture. With gcc 6.3 and 7.1 the samples are not working anymore, because the Linux system userland does not boot. Qemu 2.9.0: Kernel panic - not syncing: Attempted to kill init! exitcode=0x000b (with glibc, musl and uClibc-ng toolchains) I bisected gcc source code and found the bad commit: 6dcad60c0ef48af584395a40feeb256fb82986a8 When reverting the change, gcc 6.3 and 7.1 produces working Linux rootfs again. What can we do about it? I will revert this commit in GCC. Reverted on GCC mainline: Committed revision 248540. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: regression for microblaze architecture
On 05/27/2017 09:09 AM, Michael Eager wrote: On 05/27/2017 01:51 AM, Waldemar Brodkorb wrote: Hi, Buildroot and OpenADK have samples to create a Linux system to be bootup in Qemu system emulation for microblaze architecture. With gcc 6.3 and 7.1 the samples are not working anymore, because the Linux system userland does not boot. Qemu 2.9.0: Kernel panic - not syncing: Attempted to kill init! exitcode=0x000b (with glibc, musl and uClibc-ng toolchains) I bisected gcc source code and found the bad commit: 6dcad60c0ef48af584395a40feeb256fb82986a8 When reverting the change, gcc 6.3 and 7.1 produces working Linux rootfs again. What can we do about it? I will revert this commit in GCC. Reverted on GCC mainline: Committed revision 248540. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: regression for microblaze architecture
On 05/27/2017 01:51 AM, Waldemar Brodkorb wrote: Hi, Buildroot and OpenADK have samples to create a Linux system to be bootup in Qemu system emulation for microblaze architecture. With gcc 6.3 and 7.1 the samples are not working anymore, because the Linux system userland does not boot. Qemu 2.9.0: Kernel panic - not syncing: Attempted to kill init! exitcode=0x000b (with glibc, musl and uClibc-ng toolchains) I bisected gcc source code and found the bad commit: 6dcad60c0ef48af584395a40feeb256fb82986a8 When reverting the change, gcc 6.3 and 7.1 produces working Linux rootfs again. What can we do about it? I will revert this commit in GCC. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [PATCH] Microblaze: Fixes for RTL Checking
On 03/09/2017 03:35 PM, Segher Boessenkool wrote: Hi! On Thu, Mar 09, 2017 at 10:18:43AM -0800, Michael Eager wrote: --- a/gcc/config/microblaze/microblaze.c +++ b/gcc/config/microblaze/microblaze.c @@ -3323,10 +3323,10 @@ microblaze_expand_shift (rtx operands[]) || (GET_CODE (operands[1]) == SUBREG)); /* Shift by zero -- copy regs if necessary. */ - if ((GET_CODE (operands[2]) == CONST_INT) && (INTVAL (operands[2]) == 0)) + if (CONST_INT_P (operands[2]) && (operands[2] == const0_rtx) You don't need that first test since you have the second (and the parens are superfluous as well). Yes. Fixed. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
[PATCH] Microblaze: Fixes for RTL Checking
2017-03-09 Michael Eager <ea...@eagercon.com>
Correct failures with --enable-checking=yes,rtl.
* config/microblaze/microblaze.c (microblaze_expand_shift):
Replace GET_CODE test with CONST_INT_P and INTVAL test with
test for const0_rtx.
* config/microblaze/microblaze.md (ashlsi3_byone, ashrsi3_byone,
lshrsi3_byone): Replace INTVAL with test for const1_rtx.
diff --git a/gcc/config/microblaze/microblaze.c
b/gcc/config/microblaze/microblaze.c
index 746bef1..fb115e6 100644
--- a/gcc/config/microblaze/microblaze.c
+++ b/gcc/config/microblaze/microblaze.c
@@ -3323,10 +3323,10 @@ microblaze_expand_shift (rtx operands[])
|| (GET_CODE (operands[1]) == SUBREG));
/* Shift by zero -- copy regs if necessary. */
- if ((GET_CODE (operands[2]) == CONST_INT) && (INTVAL (operands[2]) == 0))
+ if (CONST_INT_P (operands[2]) && (operands[2] == const0_rtx)
+ && !rtx_equal_p (operands[0], operands[1]))
{
- if (REGNO (operands[0]) != REGNO (operands[1]))
- emit_insn (gen_movsi (operands[0], operands[1]));
+ emit_insn (gen_movsi (operands[0], operands[1]));
return 1;
}
diff --git a/gcc/config/microblaze/microblaze.md
b/gcc/config/microblaze/microblaze.md
index 66ebc1e..b3a0011 100644
--- a/gcc/config/microblaze/microblaze.md
+++ b/gcc/config/microblaze/microblaze.md
@@ -1321,7 +1321,7 @@
[(set (match_operand:SI 0 "register_operand" "=d")
(ashift:SI (match_operand:SI 1 "register_operand" "d")
(match_operand:SI 2 "arith_operand""I")))]
- "(INTVAL (operands[2]) == 1)"
+ "(operands[2] == const1_rtx)"
"addk\t%0,%1,%1"
[(set_attr "type""arith")
(set_attr "mode""SI")
@@ -1482,7 +1482,7 @@
[(set (match_operand:SI 0 "register_operand" "=d")
(ashiftrt:SI (match_operand:SI 1 "register_operand" "d")
(match_operand:SI 2 "arith_operand""I")))]
- "(INTVAL (operands[2]) == 1)"
+ "(operands[2] == const1_rtx)"
"sra\t%0,%1"
[(set_attr "type""arith")
(set_attr "mode""SI")
@@ -1571,7 +1571,7 @@
[(set (match_operand:SI 0 "register_operand" "=d")
(lshiftrt:SI (match_operand:SI 1 "register_operand" "d")
(match_operand:SI 2 "arith_operand""I")))]
- "(INTVAL (operands[2]) == 1)"
+ "(operands[2] == const1_rtx)"
"srl\t%0,%1"
[(set_attr "type""arith")
(set_attr "mode""SI")
Committed revision 246012.
--
Michael Eagerea...@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [www patch] sort branches
On 02/26/2017 01:38 AM, Gerald Pfeifer wrote: On Mon, 13 Feb 2017, Michael Eager wrote: The microblaze branch has been merged into the gcc mainline. Thanks for the patch, Michael. I noticed you had not committed it, so I just did that for you (with a little tweak in how we refer to GCC 4.2.1 as opposed to gcc-4.2.1). Thanks. (I don't think that I have write permission for wwwdocs.) -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [PATCH 3/6] microblaze: Fixes for RTL checking
On 02/21/2017 12:25 PM, Segher Boessenkool wrote:
On Tue, Feb 21, 2017 at 12:08:34PM -0800, Michael Eager wrote:
- /* Shift by zero -- copy regs if necessary. */
+ /* Shift by zero -- copy regs. */
if ((GET_CODE (operands[2]) == CONST_INT) && (INTVAL (operands[2]) ==
0))
{
- if (REGNO (operands[0]) != REGNO (operands[1]))
- emit_insn (gen_movsi (operands[0], operands[1]));
+ emit_insn (gen_movsi (operands[0], operands[1]));
return 1;
}
Why generate an unnecessary NOP?
Why not? It will be optimised away anyway, and the code to get at the
subregs is hairy... But could optimise away the useless move here
already if both ops are a reg and both are the same, if you prefer that?
Or rtx_equal_p (operands[0], operands[1])?
It's optimized away only if optimization is on.
The existing code checks and does not generate the unneeded move.
Whatever you change to clean up code should maintain the same behavior.
if ((operands[2] == const0_rtx) && !rtx_equal_p (operands[0], operands[1]))
{
emit_insn (gen_movsi (operands[0], operands[1]));
}
--
Michael Eagerea...@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [PATCH 3/6] microblaze: Fixes for RTL checking
On 02/21/2017 12:15 PM, Jakub Jelinek wrote: On Tue, Feb 21, 2017 at 02:48:15PM +, Segher Boessenkool wrote: - /* Shift by zero -- copy regs if necessary. */ + /* Shift by zero -- copy regs. */ if ((GET_CODE (operands[2]) == CONST_INT) && (INTVAL (operands[2]) == 0)) You could have changed this line to if (operands[2] == const0_rtx) as well. And this would not change the generated code. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [PATCH 3/6] microblaze: Fixes for RTL checking
On 02/21/2017 06:48 AM, Segher Boessenkool wrote:
REGNO can only be called on REGs, not SUBREGs; and INTVAL does not work
on REGs.
2017-02-21 Segher Boessenkool <seg...@kernel.crashing.org>
* config/microblaze/microblaze.c (microblaze_expand_shift): Do not
test for register moves to themselves.
* config/microblaze/microblaze.md (*ashlsi3_byone, *ashrsi3_byone,
*lshrsi3_byone): Test for const1_rtx instead of calling INTVAL on
something that isn't necessarily a CONST_INT.
---
gcc/config/microblaze/microblaze.c | 5 ++---
gcc/config/microblaze/microblaze.md | 6 +++---
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/gcc/config/microblaze/microblaze.c
b/gcc/config/microblaze/microblaze.c
index 746bef1..4850e85 100644
--- a/gcc/config/microblaze/microblaze.c
+++ b/gcc/config/microblaze/microblaze.c
@@ -3322,11 +3322,10 @@ microblaze_expand_shift (rtx operands[])
|| (GET_CODE (operands[1]) == REG)
|| (GET_CODE (operands[1]) == SUBREG));
- /* Shift by zero -- copy regs if necessary. */
+ /* Shift by zero -- copy regs. */
if ((GET_CODE (operands[2]) == CONST_INT) && (INTVAL (operands[2]) == 0))
{
- if (REGNO (operands[0]) != REGNO (operands[1]))
- emit_insn (gen_movsi (operands[0], operands[1]));
+ emit_insn (gen_movsi (operands[0], operands[1]));
return 1;
}
Why generate an unnecessary NOP?
--
Michael Eagerea...@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
DWARF Version 5 Standard Released
The DWARF Debugging Information Format Standards Committee is pleased to announce the availability of Version 5 of the DWARF Debugging Format Standard. The DWARF Debugging Format is used to communicate debugging information between a compiler and debugger to make it easier for programmers to develop, test, and debug programs. DWARF is used by a wide range of compilers and debuggers, both proprietary and open source, to support debugging of Ada, C, C++, Cobol, FORTRAN, Java, and other programming languages. DWARF V5 adds support for new languages like Rust, Swift, Ocaml, Go, and Haskell, as well as support for new features in older languages. DWARF can be used with a wide range of processor architectures, such as x86, ARM, PowerPC, from 8-bit to 64-bit. DWARF is the standard debugging format for Linux and several versions of Unix and is widely used with embedded processors. DWARF is designed to be extended easily to support new languages and new architectures. The DWARF Version 5 Standard has been in development for six years. DWARF Committee members include representatives from over a dozen major companies with extensive experience in compiler and debugger development. Version 5 incorporates improvements in many areas: better data compression, separation of debugging data from executable files, improved description of macros and source files, faster searching for symbols, improved debugging of optimized code, as well as numerous improvements in functionality and performance. The DWARF Version 5 Standard can be downloaded without charge from the DWARF website (dwarfstd.org). Additional information about DWARF, including how to subscribe to the DWARF mailing list, can also be found on the website. Questions about the DWARF Debugging Information Format or the DWARF Committee can be directed to the DWARF Committee Chair, Michael Eager at i...@dwarfstd.org. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [www patch] sort branches
On 02/13/2017 06:34 AM, Nathan Sidwell wrote: I've applied this patch to sort the other branch lists in svn.html. I also added an index and split the inactive branch list into merged and plain inactive. Attention branch maintainers: Please check whether I've incorrectly put a merged branch on the inactive list. nathan The microblaze branch has been merged into the gcc mainline. diff -urNp svn.html-orig svn.html --- svn.html-orig 2017-02-13 09:00:36.0 -0800 +++ svn.html2017-02-13 09:04:50.0 -0800 @@ -455,12 +455,6 @@ the command svn log --stop-on-copy are Dwarakanath Rajagopal href="mailto:dwarak.rajago...@amd.com;>dwarak.rajago...@amd.com and H.J. Lu mailto:hjl.to...@gmail.com;>hjl.to...@gmail.com. - microblaze - This branch contains support for the Xilinx MicroBlaze architecture. - This branch will be used to update MicroBlaze support from gcc-4.1.2 to - to the head. It is maintained by Michael Eager - mailto:ea...@eagercon.com;>ea...@eagercon.com. - mpx The goal of this branch is to support Intel MPX technology (href="https://software.intel.com/sites/default/files/managed/c5/15/architecture-instruction-set-extensions-programming-reference.pdf;>link). @@ -790,6 +784,13 @@ be prefixed with the initials of the dis and is currently unmaintained. This work now continues on the autovect-branch. + microblaze + This branch contained support for updating the Xilinx MicroBlaze + architecture to gcc-4.1.2. + It was created by Michael Eager + mailto:ea...@eagercon.com;>ea...@eagercon.com. + All changes have been merged into mainline. + https://gcc.gnu.org/wiki/MemRef;>mem-ref2 mips-3_4-rewrite-branch named-addr-spaces-branch -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: Make MicroBlaze support DWARF EH (old Xilinx patch, needed for glibc build)
On 01/06/2017 01:34 AM, Edgar E. Iglesias wrote: On Thu, Jan 05, 2017 at 05:58:01PM +, Joseph Myers wrote: On Thu, 5 Jan 2017, Michael Eager wrote: On multiple occasions, I have asked Xilinx to submit patches such as this one directly to the GCC/Binutils projects (assuming that they have a current FSF Copyright Assignment), or to give me explicit permission to do so on their behalf, as was the case when I originally submitted the MicroBlaze port to these projects. For whatever reason, neither has occurred. copyright.list shows assignments from Xilinx for GCC, binutils, GDB and glibc, with no indication that they are not current, or of any restriction on who at Xilinx can contribute changes. Thus, I presume we simply need someone at Xilinx to approve the contribution of this patch to GCC. The last patch sent to gcc-patches by someone @xilinx.com appears to have been <https://gcc.gnu.org/ml/gcc-patches/2016-01/msg02291.html>; author and other @xilinx.com people from that discussion CC:ed on this message. Hi Joseph, There's no problem from our side. In fact, a version of this patch has already been posted to gcc-patches: https://gcc.gnu.org/ml/gcc-patches/2013-03/msg00657.html I don't do any work on the toolchain anymore and don't have the setup to run the testsuite. Ajit or Nagaraju are might be able to help resolve any issues. Cheers, Edgar -- Joseph S. Myers jos...@codesourcery.com OK to apply. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: Make MicroBlaze support DWARF EH (old Xilinx patch, needed for glibc build)
On 01/02/2017 03:45 PM, Joseph Myers wrote: This patch, taken from <https://git.busybox.net/buildroot/tree/package/gcc/5.4.0/840-microblaze-enable-dwarf-eh-support.patch> and with a few formatting cleanups and an update for the removal of gen_rtx_raw_REG, enables DWARF EH support for MicroBlaze. This is needed for building glibc with a compiler that includes shared libgcc; right now all glibc builds for MicroBlaze are failing with my bot for lack of this support. (It's dubious if we should have glibc ports at all where required support is missing in FSF GCC.) Tested building glibc with build-many-glibcs.py. I have *not* done any other testing or any execution testing for MicroBlaze. OK to commit? I have two concerns; 1. Lack of testing on MicroBlaze. Although the patch presumably works in the Buildroot environment, that's not the same source base. I may be able to build and test this patch, but it will be a couple weeks before I can get to it. 2. Ownership and copyright. This is clearly not your authorship. Submission of a patch explicitly indicates that you are assigning your copyright interest in the patch to the FSF. I don't believe that you have copyright to this patch and can't assign it to FSF. On multiple occasions, I have asked Xilinx to submit patches such as this one directly to the GCC/Binutils projects (assuming that they have a current FSF Copyright Assignment), or to give me explicit permission to do so on their behalf, as was the case when I originally submitted the MicroBlaze port to these projects. For whatever reason, neither has occurred. The first issue is procedural/technical and easily resolved. The second issue involves Copyright Law. IANAL, but my understanding is that a third party cannot take a patch from a non-FSF/GNU repository and apply it to an FSF/GNU repository without the authors' agreement and assignment of copyright. (If Buildroot were an FSF/GNU project, then copyright would have already been assigned to FSF, presumably, and accepting the patch into GCC would not involve any transfer of ownership.) Does anyone have any authority on this copyright issue? -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [PATCH] fix linker name for uClibc
On 10/31/2016 08:04 AM, Jeff Law wrote:
On 10/28/2016 03:30 PM, Michael Eager wrote:
On 10/28/2016 11:14 AM, Waldemar Brodkorb wrote:
Hi,
uClibc-ng can be used for Microblaze architecture.
It is regulary tested with qemu-system-microblaze in little and
big endian mode.
2016-10-28 Waldemar Brodkorb <w...@uclibc-ng.org>
gcc/
* config/microblaze/linux.h: add UCLIBC_DYNAMIC_LINKER
diff --git a/gcc/config/microblaze/linux.h
b/gcc/config/microblaze/linux.h
index ae8523c..b3bf43a 100644
--- a/gcc/config/microblaze/linux.h
+++ b/gcc/config/microblaze/linux.h
@@ -29,6 +29,7 @@
#define TLS_NEEDS_GOT 1
#define GLIBC_DYNAMIC_LINKER "/lib/ld.so.1"
+#define UCLIBC_DYNAMIC_LINKER "/lib/ld-uClibc.so.0"
#if TARGET_BIG_ENDIAN_DEFAULT == 0 /* LE */
#define MUSL_DYNAMIC_LINKER_E "%{mbig-endian:;:el}"
best regards
Waldemar
OK to apply.
Thanks. I don't believe Waldemar has write access, so I committed the patch
for him.
Thanks.
--
Michael Eagerea...@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [PATCH] fix linker name for uClibc
On 10/28/2016 11:14 AM, Waldemar Brodkorb wrote:
Hi,
uClibc-ng can be used for Microblaze architecture.
It is regulary tested with qemu-system-microblaze in little and
big endian mode.
2016-10-28 Waldemar Brodkorb <w...@uclibc-ng.org>
gcc/
* config/microblaze/linux.h: add UCLIBC_DYNAMIC_LINKER
diff --git a/gcc/config/microblaze/linux.h b/gcc/config/microblaze/linux.h
index ae8523c..b3bf43a 100644
--- a/gcc/config/microblaze/linux.h
+++ b/gcc/config/microblaze/linux.h
@@ -29,6 +29,7 @@
#define TLS_NEEDS_GOT 1
#define GLIBC_DYNAMIC_LINKER "/lib/ld.so.1"
+#define UCLIBC_DYNAMIC_LINKER "/lib/ld-uClibc.so.0"
#if TARGET_BIG_ENDIAN_DEFAULT == 0 /* LE */
#define MUSL_DYNAMIC_LINKER_E "%{mbig-endian:;:el}"
best regards
Waldemar
OK to apply.
--
Michael Eagerea...@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
DWARF Version 5 Public Review Draft Released
DWARF Version 5 Public Review Draft Released October 14, 2016 The DWARF Debugging Information Format Committee has released the public review draft of Version 5 of the DWARF Debugging Information Format standard. The DWARF debugging format is used to communicate debugging information between a compiler and debugger to make it easier for programmers to develop, test, and debug programs. DWARF is used by a wide range of compilers and debuggers, both proprietary and open source, to support debugging of Ada, C, C++, Cobol, FORTRAN, Java, and other programming languages. DWARF V5 adds support for new languages like Rust, Swift, Ocaml, Go, and Haskell, as well as support for new features in the older languages. DWARF can be used with many processor architectures, from 8-bit to 64-bit. DWARF is the standard debugging format for Linux and several versions of Unix and is widely used with embedded processors. DWARF is designed to be extended easily to support new languages and new architectures. The DWARF Version 5 Standard has been in development for six years. DWARF Committee members include representatives from over a dozen major companies with extensive experience in compiler and debugger development. Version 5 incorporates improvements in many areas: better data compression, separation of debugging data from executable files, improved description of macros and source files, faster searching for symbols, improved debugging optimized code, as well as numerous improvements in functionality and performance. The public review draft of DWARF Version 5 standard can be downloaded without charge from the DWARF website (http://dwarfstd.org). The DWARF Committee will accept public comments on DWARF Version 5 until November 30, after which a finalized version will be published. Additional information about DWARF, including how to subscribe to the DWARF mailing list, can also be found on the website. Questions about the DWARF Debugging Information Format or the DWARF Committee can be directed to the DWARF Committee Chair, Michael Eager at i...@dwarfstd.org. -- Michael Eager, Chair, DWARF Debugging Format Standards Committee i...@dwarfstd.org 650-325-8077
Re: [PATCH] microblaze.c: fix warnings
OK to apply,
On 06/01/2016 01:04 PM, David Malcolm wrote:
The two microblaze configurations in contrib/config-list.mk
microblaze-elf
microblaze-linux
currently fail to build due to warnings:
microblaze.c: In function 'void insert_wic_for_ilb_runout(rtx_insn*)':
microblaze.c:3653:7: error: unused variable 'wic_addr1'
[-Werror=unused-variable]
int wic_addr1 = 128 * 4;
^
microblaze.c: In function 'void insert_wic()':
microblaze.c:3696:10: error: unused variable 'j' [-Werror=unused-variable]
int i, j;
^
microblaze.c: In function 'rtx_def* get_branch_target(rtx)':
microblaze.c:3627:1: error: control reaches end of non-void function
[-Werror=return-type]
}
^
(tested with compiling trunk using gcc 6).
It looks like these warnings were introduced in this last commit to
microblaze.c, r232683:
2016-01-21 Ajit Agarwal <ajit...@xilinx.com>
* config/microblaze/microblaze.c
(get_branch_target): New.
(insert_wic_for_ilb_runout): New.
(insert_wic): New.
(microblaze_machine_dependent_reorg): New.
(TARGET_MACHINE_DEPENDENT_REORG): Define macro.
* config/microblaze/microblaze.md
(UNSPEC_IPREFETCH): Define.
(iprefetch): New pattern
* config/microblaze/microblaze.opt
(mxl-prefetch): New flag.
The attached patch fixes them.
I picked NULL_RTX as a return value for get_branch_target; is this
sane? The result is only ever assigned to the local "branch_target"
here:
3732 if ((branch_target = get_branch_target (insn)))
and that local appears to be unused otherwise.
OK for trunk? (I've only verified that it compiles and fixes the
warnings; I haven't tested the results).
gcc/ChangeLog:
* config/microblaze/microblaze.c (get_branch_target): Add return
NULL_RTX for the non-CALL_P case.
(insert_wic_for_ilb_runout): Remove unused local "wic_addr1".
(insert_wic): Remove unused local "j".
---
gcc/config/microblaze/microblaze.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/gcc/config/microblaze/microblaze.c
b/gcc/config/microblaze/microblaze.c
index baff67a..8f4768e 100644
--- a/gcc/config/microblaze/microblaze.c
+++ b/gcc/config/microblaze/microblaze.c
@@ -3624,6 +3624,8 @@ get_branch_target (rtx branch)
abort ();
return XEXP (XEXP (call, 0), 0);
}
+
+ return NULL_RTX;
}
/* Heuristics to identify where to insert at the
@@ -3650,7 +3652,6 @@ insert_wic_for_ilb_runout (rtx_insn *first)
int addr_offset = 0;
int length;
int wic_addr0 = 128 * 4;
- int wic_addr1 = 128 * 4;
int first_addr = INSN_ADDRESSES (INSN_UID (first));
@@ -3693,7 +3694,7 @@ static void
insert_wic (void)
{
rtx_insn *insn;
- int i, j;
+ int i;
basic_block bb, prev = 0;
rtx branch_target = 0;
--
Michael Eagerea...@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: Machine constraints list
On 05/08/2016 03:27 PM, David Wohlferd wrote: Looking at the v6 release criteria (https://gcc.gnu.org/gcc-6/criteria.html) there are about a dozen supported platforms. Looking at the Machine Constraints docs (https://gcc.gnu.org/onlinedocs/gcc/Machine-Constraints.html), there are 34 architectures listed. That's a lot of entries to scroll thru. If these architectures aren't supported anymore, is it time to drop some of these from this page? I don't believe that the listing of Primary or Secondary Platforms should be considered an exhaustive list of the supported architectures. As a first pass, maybe something like this: ... > Drop ... MicroBlaze—config/microblaze/constraints.md MicroBlaze should continue to be supported. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [Patch,microblaze]: Better register allocation to minimize the spill and fetch.
On 01/29/2016 02:31 AM, Ajit Kumar Agarwal wrote: This patch improves the allocation of registers in the given function. The allocation is optimized for the conditional branches. The temporary register used in the conditional branches to store the comparison results and use of temporary in the conditional branch is optimized. Such temporary registers are allocated with a fixed register r18. Currently such temporaries are allocated with a free registers in the given function. Due to this one of the free register is reserved for the temporaries and given function is left with a few registers. This is unoptimized with respect to microblaze. In Microblaze r18 is marked as fixed and cannot be allocated to pseudos' in the given function. Instead r18 can be used as a temporary for the conditional branches with compare and branch. Use of r18 as a temporary for conditional branches will save one of the free registers to be allocated. The free registers can be used for other pseudos' and hence the better register allocation. The usage of r18 as above reduces the spill and fetch because of the availability of one of the free registers to other pseudos instead of being used for conditional temporaries. The advantage of the above is that the scope of the temporaries is limited to the conditional branches and hence the usage of r18 as temporary for such conditional branches is optimized and preserve the functionality of the function. Regtested for Microblaze target. Performance runs are done with Mibench/EEMBC benchmarks. Following gains are achieved. Benchmarks Gains automotive_qsort1 1.630730524% network_dijkstra 1.527506256% office_stringsearch 1 1.81356288% security_rijndael_d 3.26129357% basefp01_lite 4.465120185% a2time01_lite 1.893862857% cjpeg_lite 3.286496675% djpeg_lite 3.120150612% qos_lite 2.63964381% office_ispell 1.531340405% Code Size improvements: Reduction in number of instructions for Mibench : 12927. Reduction in number of instructions for EEMBC : 212. ChangeLog: 2016-01-29 Ajit Agarwal <ajit...@xilinx.com> * config/microblaze/microblaze.c (microblaze_expand_conditional_branch): Use of MB_ABI_ASM_TEMP_REGNUM for temporary conditional branch. (microblaze_expand_conditional_branch_reg): Use of MB_ABI_ASM_TEMP_REGNUM for temporary conditional branch. (microblaze_expand_conditional_branch_sf): Use of MB_ABI_ASM_TEMP_REGNUM for temporary conditional branch. You can combine these ChangeLog entries: * config/microblaze/microblaze.c (microblaze_expand_conditional_branch, microblaze_expand_conditional_branch_reg, microblaze_expand_conditional_branch_sf): Use MB_ABI_ASM_TEMP_REGNUM for temp reg. Otherwise, OK. Signed-off-by:Ajit Agarwal ajit...@xilinx.com. --- gcc/config/microblaze/microblaze.c |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/gcc/config/microblaze/microblaze.c b/gcc/config/microblaze/microblaze.c index baff67a..b4277ad 100644 --- a/gcc/config/microblaze/microblaze.c +++ b/gcc/config/microblaze/microblaze.c @@ -3402,7 +3402,7 @@ microblaze_expand_conditional_branch (machine_mode mode, rtx operands[]) rtx cmp_op0 = operands[1]; rtx cmp_op1 = operands[2]; rtx label1 = operands[3]; - rtx comp_reg = gen_reg_rtx (SImode); + rtx comp_reg = gen_rtx_REG (SImode, MB_ABI_ASM_TEMP_REGNUM); rtx condition; gcc_assert ((GET_CODE (cmp_op0) == REG) || (GET_CODE (cmp_op0) == SUBREG)); @@ -3439,7 +3439,7 @@ microblaze_expand_conditional_branch_reg (enum machine_mode mode, rtx cmp_op0 = operands[1]; rtx cmp_op1 = operands[2]; rtx label1 = operands[3]; - rtx comp_reg = gen_reg_rtx (SImode); + rtx comp_reg = gen_rtx_REG (SImode, MB_ABI_ASM_TEMP_REGNUM); rtx condition; gcc_assert ((GET_CODE (cmp_op0) == REG) @@ -3483,7 +3483,7 @@ microblaze_expand_conditional_branch_sf (rtx operands[]) rtx condition; rtx cmp_op0 = XEXP (operands[0], 0); rtx cmp_op1 = XEXP (operands[0], 1); - rtx comp_reg = gen_reg_rtx (SImode); + rtx comp_reg = gen_rtx_REG (SImode, MB_ABI_ASM_TEMP_REGNUM); emit_insn (gen_cstoresf4 (comp_reg, operands[0], cmp_op0, cmp_op1)); condition = gen_rtx_NE (SImode, comp_reg, const0_rtx); -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [Patch,microblaze]: Optimized register reorganization for Microblaze.
On 01/12/2016 09:42 AM, Ajit Kumar Agarwal wrote:
The patch contains the changes in the macros fixed_registers and
call_used_registers.
Earlier the register r21 is marked as fixed and also marked as 1 for call_used
registers.
On top of that r21 is not assigned to any of the temporaries in rtl insns.
This makes the usage of registers r21 in the callee functions not possible and
wasting
one registers to allocate in the callee function. The changes makes the
register r21 as
allocatable to the callee function and optimized usage of the registers r21 in
the callee
function reduces spill and fetch. The reduction in the spill and fetch is due
to availability
of register r21 in the callee function. The availability of register r21 is
made by
marking the register r21 as not fixed and the call_used registers is marked as
0.
Also r20 is marked as fixed. The changes are done not to mark as fixed thus
allowing
the registers to be used reducing the spill and fetch.
Regtested for Microblaze.
Performance runs made on Mibench/EEMBC benchmarks for microblaze. Following
benchmarks
shows the gains
Benchmarks Gains
automotive_qsort1 =3.96%
automotive_susan_c = 7.68%
consumer_mad =9.6%
security_rijndael_d =19.57%
telecom_CRC32 = 7.66%
bitmnp01_lite = 10.61%
a2time01_lite =6.97%
ChangeLog:
2016-01-12 Ajit Agarwal <ajit...@xilinx.com>
* config/microblaze/microblaze.h
(FIXED_REGISTERS): Update in macro.
(CALL_USED_REGISTERS): Update in macro.
Signed-off-by:Ajit Agarwal ajit...@xilinx.com.
---
gcc/config/microblaze/microblaze.h |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/gcc/config/microblaze/microblaze.h
b/gcc/config/microblaze/microblaze.h
index e115c42..dbfb652 100644
--- a/gcc/config/microblaze/microblaze.h
+++ b/gcc/config/microblaze/microblaze.h
@@ -253,14 +253,14 @@ extern enum pipeline_type microblaze_pipe;
#define FIXED_REGISTERS
\
{ \
1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, \
- 1, 1, 1, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
+ 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
1, 1, 1, 1 \
}
#define CALL_USED_REGISTERS \
{ \
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, \
- 1, 1, 1, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
+ 1, 1, 1, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
1, 1, 1, 1 \
}
#define GP_REG_FIRST0
Committed revision 232682.
--
Michael Eagerea...@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [Patch,microblaze]: Instruction prefetch optimization for microblaze.
On 12/07/2015 09:39 AM, Ajit Kumar Agarwal wrote: -Original Message- From: Michael Eager [mailto:ea...@eagerm.com] Sent: Thursday, December 03, 2015 7:27 PM To: Ajit Kumar Agarwal; GCC Patches Cc: Vinod Kathail; Shail Aditya Gupta; Vidhumouli Hunsigida; Nagaraju Mekala Subject: Re: [Patch,microblaze]: Instruction prefetch optimization for microblaze. On 12/01/2015 12:49 AM, Ajit Kumar Agarwal wrote: The changes are made in this patch for the instruction prefetch optimizations for Microblaze. Reg tested for Microblaze target. The changes are made for instruction prefetch optimizations for Microblaze. The "wic" microblaze instruction is the instruction prefetch instruction. The instruction prefetch optimization is done to generate the iprefetch instruction at the call site fall through path. This optimization is enabled with microblaze target flag mxl-prefetch. The purpose of adding the flags is that selection of "wic" instruction should be enabled in the reconfigurable design and the selection is not enabled by default. ChangeLog: 2015-12-01 Ajit Agarwal <ajit...@xilinx.com> * config/microblaze/microblaze.c (get_branch_target): New. (insert_wic_for_ilb_runout): New. (insert_wic): New. (microblaze_machine_dependent_reorg): New. (TARGET_MACHINE_DEPENDENT_REORG): Define macro. * config/microblaze/microblaze.md (UNSPEC_IPREFETCH): Define. (iprefetch): New pattern * config/microblaze/microblaze.opt (mxl-prefetch): New flag. Signed-off-by:Ajit Agarwal ajit...@xilinx.com Thanks & Regards Ajit + rtx_insn *insn, *before_4 = 0, *before_16 = 0; int addr = 0, length, + first_addr = -1; int wic_addr0 = 128 * 4, wic_addr1 = 128 * 4; Especially when there are initializers, I prefer to see each variable declared on a separate line. If the meaning of a variable is not clear (and most of these are not), include a comment >>before the declaration. +if (first_addr == -1) + first_addr = INSN_ADDRESSES (INSN_UID (insn)); Can be moved to initialize first_addr. +addr = INSN_ADDRESSES (INSN_UID (insn)) - first_addr; Is "addr" and address or offset? If the latter, use a more descriptive name. +if (before_4 == 0 && addr + length >= 4 * 4) + before_4 = insn; ... Please add comments to describe what you are doing here. What are before_4 and before_16? What are all these conditions testing? + loop_optimizer_finalize(); Space before parens. All the above comments are incorporated. Updated patch is attached. Regtested for Microblaze target. Mibench/EEMBC benchmarks are run on the hardware enabling the mxl-prefetch and the run goes through fine With the generation of "wic" instruction. [Patch,microblaze]: Instruction prefetch optimization for microblaze. The changes are made for instruction prefetch optimizations for Microblaze. The "wic" microblaze instruction is the instruction prefetch instruction. The instruction prefetch optimization is done to generate the iprefetch instruction at the call site fall through path. This optimization is enabled with microblaze target flag mxl-prefetch. The purpose of adding the flags is that selection of "wic" instruction should be enabled in the reconfigurable design and the selection is not enabled by default. ChangeLog: 2015-12-07 Ajit Agarwal <ajit...@xilinx.com> * config/microblaze/microblaze.c (get_branch_target): New. (insert_wic_for_ilb_runout): New. (insert_wic): New. (microblaze_machine_dependent_reorg): New. (TARGET_MACHINE_DEPENDENT_REORG): Define macro. * config/microblaze/microblaze.md (UNSPEC_IPREFETCH): Define. (iprefetch): New pattern * config/microblaze/microblaze.opt (mxl-prefetch): New flag. Signed-off-by:Ajit Agarwal ajit...@xilinx.com Thanks & Regards Ajit Committed revision 232683. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [doc, committed] document MicroBlaze interrupt_handler and fast_interrupt attributes
On 01/05/2016 10:08 AM, Sandra Loosemore wrote: I've checked in this patch to add some minimal documentation for the MicroBlaze interrupt handler attributes. I was able to figure this out by inspection of the code and reading the processor documentation, which has a more extensive discussion of how to use these features. -Sandra OK. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [Patch,microblaze]: Instruction prefetch optimization for microblaze.
On 12/01/2015 12:49 AM, Ajit Kumar Agarwal wrote: The changes are made in this patch for the instruction prefetch optimizations for Microblaze. Reg tested for Microblaze target. The changes are made for instruction prefetch optimizations for Microblaze. The "wic" microblaze instruction is the instruction prefetch instruction. The instruction prefetch optimization is done to generate the iprefetch instruction at the call site fall through path. This optimization is enabled with microblaze target flag mxl-prefetch. The purpose of adding the flags is that selection of "wic" instruction should be enabled in the reconfigurable design and the selection is not enabled by default. ChangeLog: 2015-12-01 Ajit Agarwal <ajit...@xilinx.com> * config/microblaze/microblaze.c (get_branch_target): New. (insert_wic_for_ilb_runout): New. (insert_wic): New. (microblaze_machine_dependent_reorg): New. (TARGET_MACHINE_DEPENDENT_REORG): Define macro. * config/microblaze/microblaze.md (UNSPEC_IPREFETCH): Define. (iprefetch): New pattern * config/microblaze/microblaze.opt (mxl-prefetch): New flag. Signed-off-by:Ajit Agarwal ajit...@xilinx.com Thanks & Regards Ajit + rtx_insn *insn, *before_4 = 0, *before_16 = 0; + int addr = 0, length, first_addr = -1; + int wic_addr0 = 128 * 4, wic_addr1 = 128 * 4; Especially when there are initializers, I prefer to see each variable declared on a separate line. If the meaning of a variable is not clear (and most of these are not), include a comment before the declaration. +if (first_addr == -1) + first_addr = INSN_ADDRESSES (INSN_UID (insn)); Can be moved to initialize first_addr. +addr = INSN_ADDRESSES (INSN_UID (insn)) - first_addr; Is "addr" and address or offset? If the latter, use a more descriptive name. +if (before_4 == 0 && addr + length >= 4 * 4) + before_4 = insn; ... Please add comments to describe what you are doing here. What are before_4 and before_16? What are all these conditions testing? + loop_optimizer_finalize(); Space before parens. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [PING][PATCH 5/13] microblaze musl support
On 10/15/2015 12:00 PM, Szabolcs Nagy wrote: On 06/05/15 12:25, Szabolcs Nagy wrote: On 29/04/15 14:51, Szabolcs Nagy wrote: On 29/04/15 14:17, Michael Eager wrote: On 04/27/2015 07:35 AM, Szabolcs Nagy wrote: On 20/04/15 19:54, Szabolcs Nagy wrote: Set up dynamic linker name for microblaze. Patch v2. (undef MUSL_DYNAMIC_LINKER that comes from config/linux.h) Are you building with both glibc and musl to verify these patches? ... note that microblaze does not use the GNU_USER_DYNAMIC_LINKER macro so the -mglibc etc options don't work. (that should be changed probably, assuming -muclibc and -mbionic have no side effects when they are not supported) Patch v3. I changed the patch to use GNU_USER_DYNAMIC_LINKER like other platforms do. I built gcc with musl and glibc default libc as well and verified that -mmusl and -mglibc sets the dynamic linker as expected. Ping, patch still works. https://gcc.gnu.org/ml/gcc-patches/2015-05/msg00437.html gcc/Changelog: 2015-05-06 Gregor Richards <gregor.richa...@uwaterloo.ca> Szabolcs Nagy <szabolcs.n...@arm.com> * config/microblaze/linux.h (MUSL_DYNAMIC_LINKER): Define. (DYNAMIC_LINKER): Renamed to ... (GLIBC_DYNAMIC_LINKER): This. (SUBTARGET_EXTRA_SPECS): Use GNU_USER_DYNAMIC_LINKER. OK. Do you have commit privileges? -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [PATCH] Fix building microblaze targets with trunk
On 09/29/2015 10:01 AM, Jeff Law wrote: The microblaze port as a "*p++" statement which computes a result that is never used (the memory result). This removes the spurious memory dereference and the unused value warning. Tested by building the microblaze targets in config-all.mk. Installed on the trunk. OK. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [DWARF] Tracking uninitialized variables
On 07/20/2015 09:55 AM, Nikolai Bozhenov wrote: On 07/17/2015 08:31 PM, Michael Eager wrote: A related issue is where the breakpoint is taken. GCC sets breakpoints at the first instruction generated for a statement, which in this case, appears to be before any of the arguments to bar are evaluated. A possibly better location would be after arguments are evaluated, before the call is executed. In this case GDB set the breakpoint at the instruction at 0x0d where evaluation of the first argument for the call is performed. I'm not sure that there is a less confusing way to choose an address to set a breakpoint. For example I don't think it is a good idea to ignore evaluation of function arguments and set a breakpoint right at the call instruction. But even if there is a better way, such new feature is likely to be implemented in GDB rather than in GCC. Debugging optimized programs is difficult, and deciding the best location to set a breakpoint is a matter for some thought. (See Caroline Tice's dissertation.) If none of the arguments is evaluated until after the first instruction of a function call, then you cannot display the argument values if that is your breakpoint. No matter what location you feel is the best for a breakpoint, it is the compiler which generates the line number table and indicates where a breakpoint should be placed, not the debugger. Don't expect anything in GDB which will do something when GCC provides incomplete or inaccurate information. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [DWARF] Tracking uninitialized variables
On 07/17/2015 03:43 AM, Nikolai Bozhenov wrote:
Hello!
It is certainly true that debugging an optimized code is an inherently
difficult task. Though, I wonder if the compiler could make such
debugging experience slightly less surprising.
Consider the following example:
1 extern void bar(int *i1, int *i2, int *i3);
2
3 int __attribute__((noinline)) foo(int i1, int i2) {
4 int a, b, c;
5 a = i1 i2;
6 b = (i1 + i2) * i1;
7 c = (b + i1);
8 bar(a, b, c);
9 }
10
11 int main() {
12 foo(42, 12);
13 }
Let's compile it:
$ gcc-trunk tst.c -g -fvar-tracking-uninit -O2
After hitting a breakpoint at line 8 (the last line of the function
foo) I have some random (and very confusing) values displayed in gdb
for all three variables a, b and c. This is because GCC allocates
these three variables on the stack (their addresses are taken) and
creates for them DWARF entries like this:
2a8: Abbrev Number: 8 (DW_TAG_variable)
a9 DW_AT_name: a
ab DW_AT_decl_file : 1
ac DW_AT_decl_line : 4
ad DW_AT_type: 0x64
b1 DW_AT_location: 2 byte block: 91 64 (DW_OP_fbreg: -28)
This (incorrectly) says that the variable is at the specified location
for the entire scope of the function. This should be a location list,
which specifies the live range for the variable. At the breakpoint,
this location list would/should have no location for the variable.
A related issue is where the breakpoint is taken. GCC sets breakpoints
at the first instruction generated for a statement, which in this case,
appears to be before any of the arguments to bar are evaluated. A
possibly better location would be after arguments are evaluated, before
the call is executed.
That is, actual values for variables are supposed to reside in fixed
stack slots throughout the whole function. But in fact, by the time
the breakpoint is hit none of the values is stored on the stack (the
last line of the function, evaluation of the first argument for the
call):
Dump of assembler code for function foo:
0x004004c0 +0: mov%esi,%ecx
0x004004c2 +2: add%edi,%esi
0x004004c4 +4: sub$0x18,%rsp
0x004004c8 +8: imul %edi,%esi
0x004004cb +11:mov%edi,%eax
= 0x004004cd +13:lea0xc(%rsp),%rdx
0x004004d2 +18:shl%cl,%eax
0x004004d4 +20:mov%eax,0x4(%rsp)
0x004004d8 +24:mov%esi,0x8(%rsp)
0x004004dc +28:add%edi,%esi
0x004004de +30:lea0x4(%rsp),%rdi
0x004004e3 +35:mov%esi,0xc(%rsp)
0x004004e7 +39:lea0x8(%rsp),%rsi
0x004004ec +44:callq 0x4004f6 bar
0x004004f1 +49:add$0x18,%rsp
0x004004f5 +53:retq
End of assembler dump.
By contrast, If I didn't take addresses of these variables, they would
be optimized out until we can determine their correct values. I
believe that such behavior is much better than displaying wrong values
for variables. Also, GCC seems to be able to produce DW_OP_GNU_uninit
operations to mark uninitialized variables (when -fvar-tracking-uninit
is used) but I've never seen it in generated debug info.
At first glance the problem seems to have something to do with the
fact that there's no gimple_debug/BIND and debug_insn/var_location
instructions for such stack variables in internal representation.
There are only statements like this:
gimple_assign lshift_expr, a.0_3, i1_1(D), i2_2(D), NULL
# .MEM_5 = VDEF .MEM_4(D)
gimple_assign ssa_name, a, a.0_3, NULL, NULL
and *no* BIND statement like gimple_debug BIND a, a.0_3
Furthermore, in dwarf2out.c there are a lot of calls to
mem_loc_descriptor where var_init_status is unconditionally set to
VAR_INIT_STATUS_INITIALIZED. And it looks like currently all values
in memory are considered to be initialized by default. And only in
few places NOTE_VAR_LOCATION_STATUS is taken into account (anyway,
there's no such notes for variables a, b, c in this case).
So, here are my questions:
1) Wouldn't it be nice if GCC marked such stack variables either as
uninitialized (with DW_OP_GNU_uninit) or as optimized out?
2) Is it feasible to implement such tracking for variables?
3) What exactly is GCC supposed to track with -fvar-tracking-uninit
option?
Thanks,
Nikolai
--
Michael Eagerea...@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [Patch,microblaze]: Optimized usage of fint instruction.
On 03/04/2015 08:20 AM, Michael Eager wrote: On 03/04/15 03:53, Ajit Kumar Agarwal wrote: -Original Message- From: Michael Eager [mailto:ea...@eagerm.com] Sent: Thursday, February 26, 2015 4:33 AM To: Ajit Kumar Agarwal; GCC Patches Cc: Vinod Kathail; Shail Aditya Gupta; Vidhumouli Hunsigida; Nagaraju Mekala Subject: Re: [Patch,microblaze]: Optimized usage of fint instruction. On 02/25/15 02:20, Ajit Kumar Agarwal wrote: Hello All: Please find the patch for the optimized usage of fint instruction changes. No regression is seen in the deja GNU tests. commit ed4dc0b96bf43c200cacad97f73a98ab7048e51b Author: Ajit Kumar Agarwal ajitkum@xhdspdgnu.(none) Date: Wed Feb 25 15:36:29 2015 +0530 [Patch,microblaze]: Optimized usage of fint instruction. The changes are made in the patch for optimized usage of fint instruction. The sequence of fint/cond_branch is replaced with fcmp/cond_branch. The fint instruction takes 6/7 cycles as compared to fcmp instruction which takes 1 cycles. The conversion from float to int with fint instruction is not required and can directly compared with fcmp instruction which takes 1 cycle as compared to 6/7 cycles with fint instruction. ChangeLog: 2015-02-25 Ajit Agarwal ajit...@xilinx.com * config/microblaze/microblaze.md (peephole2): New. +emit_insn (gen_cstoresf4 (comp_reg, operands[2], + gen_rtx_REG(SFmode,REGNO(cmp_op0)), + gen_rtx_REG(SFmode,REGNO(cmp_op1; Spaces before left parens and after comma in last two lines. Changes are incorporated. Please find the log for updated patch. commit 492b0d0b67a5b12d2dc239de3215630c8838edea Author: Ajit Kumar Agarwal ajitkum@xhdspdgnu.(none) Date: Wed Mar 4 17:15:16 2015 +0530 [Patch,microblaze]: Optimized usage of fint instruction. The changes are made in the patch for optimized usage of fint instruction. The sequence of fint/cond_branch is replaced with fcmp/cond_branch. The fint instruction takes 6/7 cycles as compared to fcmp instruction which takes 1 cycles. The conversion from float to int with fint instruction is not required and can directly compared with fcmp instruction which takes 1 cycle as compared to 6/7 cycles with fint instruction. ChangeLog: 2015-03-04 Ajit Agarwal ajit...@xilinx.com * config/microblaze/microblaze.md (peephole2): New. Signed-off-by:Ajit Agarwal ajit...@xilinx.com Thanks Regards Ajit OK. Committed revision 222790. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [Patch,microblaze]: Optimized usage of pcmp conditional instruction.
On 03/06/2015 07:33 AM, Michael Eager wrote: On 03/05/15 21:12, Ajit Kumar Agarwal wrote: Changes are incorporated. Please find the log of the updated patch. commit 91f275c144165320850ddf18e3a1e059a66c Author: Ajit Kumar Agarwal ajitkum@xhdspdgnu.(none) Date: Fri Mar 6 09:55:11 2015 +0530 [Patch,microblaze]: Optimized usage of pcmp conditional instruction. The changes are made in the patch for optimized usage of pcmpne/pcmpeq instructions. The xor with register to register is replaced with pcmpeq /pcmpne instructions and for immediate check still the xori will be used. The purpose of the change is to acheive the aggressive usage of pcmpne /pcmpeq instructions instead of xor being used for comparison. ChangeLog: 2015-03-06 Ajit Agarwal ajit...@xilinx.com * config/microblaze/microblaze.md (cbranchsi4): Added immediate constraints. (cbranchsi4_reg): New. * config/microblaze/microblaze.c (microblaze_expand_conditional_branch_reg): New. * config/microblaze/microblaze-protos.h (microblaze_expand_conditional_branch_reg): New prototype. Signed-off-by:Ajit Agarwal ajit...@xilinx.com Thanks Regards Ajit OK. Committed revision 222791. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [PATCH 5/13] microblaze musl support
On 04/27/2015 07:35 AM, Szabolcs Nagy wrote: On 20/04/15 19:54, Szabolcs Nagy wrote: Set up dynamic linker name for microblaze. Patch v2. (undef MUSL_DYNAMIC_LINKER that comes from config/linux.h) gcc/Changelog: 2015-04-24 Gregor Richards gregor.richa...@uwaterloo.ca * config/microblaze/linux.h (MUSL_DYNAMIC_LINKER): Define. (DYNAMIC_LINKER): Change. Are you building with both glibc and musl to verify these patches? -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [Patch,microblaze]: Optimized usage of pcmp conditional instruction.
On 03/05/15 21:12, Ajit Kumar Agarwal wrote: Changes are incorporated. Please find the log of the updated patch. commit 91f275c144165320850ddf18e3a1e059a66c Author: Ajit Kumar Agarwal ajitkum@xhdspdgnu.(none) Date: Fri Mar 6 09:55:11 2015 +0530 [Patch,microblaze]: Optimized usage of pcmp conditional instruction. The changes are made in the patch for optimized usage of pcmpne/pcmpeq instructions. The xor with register to register is replaced with pcmpeq /pcmpne instructions and for immediate check still the xori will be used. The purpose of the change is to acheive the aggressive usage of pcmpne /pcmpeq instructions instead of xor being used for comparison. ChangeLog: 2015-03-06 Ajit Agarwal ajit...@xilinx.com * config/microblaze/microblaze.md (cbranchsi4): Added immediate constraints. (cbranchsi4_reg): New. * config/microblaze/microblaze.c (microblaze_expand_conditional_branch_reg): New. * config/microblaze/microblaze-protos.h (microblaze_expand_conditional_branch_reg): New prototype. Signed-off-by:Ajit Agarwal ajit...@xilinx.com Thanks Regards Ajit OK. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [Patch,microblaze]: Optimized usage of fint instruction.
On 03/04/15 03:53, Ajit Kumar Agarwal wrote: -Original Message- From: Michael Eager [mailto:ea...@eagerm.com] Sent: Thursday, February 26, 2015 4:33 AM To: Ajit Kumar Agarwal; GCC Patches Cc: Vinod Kathail; Shail Aditya Gupta; Vidhumouli Hunsigida; Nagaraju Mekala Subject: Re: [Patch,microblaze]: Optimized usage of fint instruction. On 02/25/15 02:20, Ajit Kumar Agarwal wrote: Hello All: Please find the patch for the optimized usage of fint instruction changes. No regression is seen in the deja GNU tests. commit ed4dc0b96bf43c200cacad97f73a98ab7048e51b Author: Ajit Kumar Agarwal ajitkum@xhdspdgnu.(none) Date: Wed Feb 25 15:36:29 2015 +0530 [Patch,microblaze]: Optimized usage of fint instruction. The changes are made in the patch for optimized usage of fint instruction. The sequence of fint/cond_branch is replaced with fcmp/cond_branch. The fint instruction takes 6/7 cycles as compared to fcmp instruction which takes 1 cycles. The conversion from float to int with fint instruction is not required and can directly compared with fcmp instruction which takes 1 cycle as compared to 6/7 cycles with fint instruction. ChangeLog: 2015-02-25 Ajit Agarwal ajit...@xilinx.com * config/microblaze/microblaze.md (peephole2): New. +emit_insn (gen_cstoresf4 (comp_reg, operands[2], + gen_rtx_REG(SFmode,REGNO(cmp_op0)), + gen_rtx_REG(SFmode,REGNO(cmp_op1; Spaces before left parens and after comma in last two lines. Changes are incorporated. Please find the log for updated patch. commit 492b0d0b67a5b12d2dc239de3215630c8838edea Author: Ajit Kumar Agarwal ajitkum@xhdspdgnu.(none) Date: Wed Mar 4 17:15:16 2015 +0530 [Patch,microblaze]: Optimized usage of fint instruction. The changes are made in the patch for optimized usage of fint instruction. The sequence of fint/cond_branch is replaced with fcmp/cond_branch. The fint instruction takes 6/7 cycles as compared to fcmp instruction which takes 1 cycles. The conversion from float to int with fint instruction is not required and can directly compared with fcmp instruction which takes 1 cycle as compared to 6/7 cycles with fint instruction. ChangeLog: 2015-03-04 Ajit Agarwal ajit...@xilinx.com * config/microblaze/microblaze.md (peephole2): New. Signed-off-by:Ajit Agarwal ajit...@xilinx.com Thanks Regards Ajit OK. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [Patch,microblaze]: Optimized usage of fint instruction.
On 02/25/15 02:20, Ajit Kumar Agarwal wrote: Hello All: Please find the patch for the optimized usage of fint instruction changes. No regression is seen in the deja GNU tests. commit ed4dc0b96bf43c200cacad97f73a98ab7048e51b Author: Ajit Kumar Agarwal ajitkum@xhdspdgnu.(none) Date: Wed Feb 25 15:36:29 2015 +0530 [Patch,microblaze]: Optimized usage of fint instruction. The changes are made in the patch for optimized usage of fint instruction. The sequence of fint/cond_branch is replaced with fcmp/cond_branch. The fint instruction takes 6/7 cycles as compared to fcmp instruction which takes 1 cycles. The conversion from float to int with fint instruction is not required and can directly compared with fcmp instruction which takes 1 cycle as compared to 6/7 cycles with fint instruction. ChangeLog: 2015-02-25 Ajit Agarwal ajit...@xilinx.com * config/microblaze/microblaze.md (peephole2): New. +emit_insn (gen_cstoresf4 (comp_reg, operands[2], + gen_rtx_REG(SFmode,REGNO(cmp_op0)), + gen_rtx_REG(SFmode,REGNO(cmp_op1; Spaces before left parens and after comma in last two lines. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [Patch,microblaze]: Optimized usage of pcmp conditional instruction.
On 02/25/15 02:19, Ajit Kumar Agarwal wrote:
Hello All:
Please find the patch for the optimized usage of pcmp instructions in
microblaze. No regressions is seen
In deja GNU tests. There are many testcases that are already there in deja GNU
to check the generation of
pcmpne/pcmpeq instructions and are used to check the validity.
commit b74acf44ce4286649e5be7cff7518d814cb2491f
Author: Ajit Kumar Agarwal ajitkum@xhdspdgnu.(none)
Date: Wed Feb 25 15:33:02 2015 +0530
[Patch,microblaze]: Optimized usage of pcmp conditional instruction.
The changes are made in the patch for optimized usage of pcmpne/pcmpeq
instructions. The xor with register to register is replaced with pcmpeq
/pcmpne instructions and for immediate check still the xori will be used.
The purpose of the change is to acheive the aggressive usage of pcmpne
/pcmpeq instructions instead of xor being used for comparison.
ChangeLog:
2015-02-25 Ajit Agarwal ajit...@xilinx.com
* config/microblaze/microblaze.md (cbranchsi4): Added immediate
constraints.
(cbranchsi4_reg): New.
* config/microblaze/microblaze.c
(microblaze_expand_conditional_branch_reg): New.
* config/microblaze/microblaze-protos.h
(microblaze_expand_conditional_branch_reg): New prototype.
+ if (cmp_op1 == const0_rtx)
+{
+ comp_reg = cmp_op0;
+ condition = gen_rtx_fmt_ee (signed_condition (code),
+ SImode, comp_reg, const0_rtx);
+ emit_jump_insn (gen_condjump (condition, label1));
+}
+
+ else if (code == EQ || code == NE)
+{
+ if (code == NE)
+{
+ emit_insn (gen_sne_internal_pat (comp_reg, cmp_op0,
+ cmp_op1));
+ condition = gen_rtx_NE (SImode, comp_reg, const0_rtx);
+}
+ else
+{
+ emit_insn (gen_seq_internal_pat (comp_reg,
+ cmp_op0, cmp_op1));
+ condition = gen_rtx_EQ (SImode, comp_reg, const0_rtx);
+}
+ emit_jump_insn (gen_condjump (condition, label1));
+}
+ else
+{
...
No blank line between end brace of if and else.
Replace with
+ else if (code == EQ)
+{
+ emit_insn (gen_seq_internal_pat (comp_reg, cmp_op0, cmp_op1));
+ condition = gen_rtx_EQ (SImode, comp_reg, const0_rtx);
+ emit_jump_insn (gen_condjump (condition, label1));
+}
+ else if (code == NE)
+{
+ emit_insn (gen_sne_internal_pat (comp_reg, cmp_op0, cmp_op1));
+ condition = gen_rtx_NE (SImode, comp_reg, const0_rtx);
+ emit_jump_insn (gen_condjump (condition, label1));
+}
+ else
+{
...
--
Michael Eagerea...@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [PATCH] microblaze: microblaze.md: Use 'SI' instead of 'VOID' for operand 1 of 'call_value_intern'
On 12/20/14 02:09, Chen Gang wrote: By the way, if this thread really has negative effect with other members, please warn me, I should not notify it to mailing list again, and try my best to finish it within myself. I appreciate your enthusiasm and perseverance in pursuing this bug. If the problem you are working on has changed from the mb-gcc issue, change the subject. Otherwise, keep up the good work. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [PATCH] DWARF add DW_AT_noreturn on noreturn function subprogram.
On 12/01/14 14:02, Cary Coutant wrote: [+cc Michael Eager] Rather than having to lobby to keep it unchanged because we jumped the gun, can we lobby to get the number assigned in the near future rather than in the potentially far future? That feels more cooperative to me :-) Would that make Michael happier? I'm pretty confident that Michael will say, don't rely on the value until the final spec is published. (He's told me exactly that in the past. I've been guilty of jumping the gun on a couple of DW_LANG codes and a few DW_AT values.) But we should let Michael answer for himself... As Cary said, the values are not fixed until the spec is released. As Jason said, the values are unlikely to change. The standards process can be a bit messy at times, particularly when we realize that something we added to a draft needs to be changed. We try to avoid making unnecessary changes, naturally. On the other hand, we don't want something in an internal draft of the DWARF specification to prevent us from making necessary changes. We will avoid changing the value of the DW_AT_noreturn or any other new attribute, if for no reason than it makes additional work for us. We're also aware that changes may affect work on GCC, GDB, or other tools, which we would like to avoid. The most conservative approach is to wait for the DWARF Version 5 spec to be released. While there is no guarantee that attribute values will not change in the final spec, the risk if this happening seems small. Michael, for your reference, here's the start of this thread: https://gcc.gnu.org/ml/gcc-patches/2014-11/msg03195.html and its continuation into December: https://gcc.gnu.org/ml/gcc-patches/2014-12/msg00099.html Michael, are you OK with sharing your target dates for publishing the spec? We will likely have a public review draft available in April or May with a 60 period for comments, with final release most likely in July. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [microblaze] RFA: Use new rtl iterators in microblaze_tls_referenced_p
On 10/25/14 03:05, Richard Sandiford wrote:
This is part of a series to remove uses of for_each_rtx from the ports.
Tested by making sure there were no code changes for gcc.dg, gcc.c-torture
and g++.dg for microblaze-elf. OK to install?
Yes, this is OK. Please check for trailing whitespace.
Thanks,
Richard
gcc/
* config/microblaze/microblaze.c: Include rtl-iter.h.
(microblaze_tls_referenced_p_1): Delete.
(microblaze_tls_referenced_p): Use FOR_EACH_SUBRTX.
Index: gcc/config/microblaze/microblaze.c
===
--- gcc/config/microblaze/microblaze.c 2014-10-25 09:40:37.967516501 +0100
+++ gcc/config/microblaze/microblaze.c 2014-10-25 09:51:27.863905096 +0100
@@ -56,6 +56,7 @@
#include diagnostic-core.h
#include cgraph.h
#include builtins.h
+#include rtl-iter.h
#define MICROBLAZE_VERSION_COMPARE(VA,VB) strcasecmp (VA, VB)
@@ -444,20 +445,6 @@ microblaze_tls_symbol_p (rtx x)
return SYMBOL_REF_TLS_MODEL (x) != 0;
}
-static int
-microblaze_tls_operand_p_1 (rtx *x, void *data ATTRIBUTE_UNUSED)
-{
- if (GET_CODE (*x) == SYMBOL_REF)
-return SYMBOL_REF_TLS_MODEL (*x) != 0;
-
- /* Don't recurse into UNSPEC_TLS looking for TLS symbols; these are
- TLS offsets, not real symbol references. */
- if (GET_CODE (*x) == UNSPEC XINT (*x, 1) == UNSPEC_TLS)
-return -1;
-
- return 0;
-}
-
/* Return TRUE if X contains any TLS symbol references. */
bool
@@ -465,8 +452,18 @@ microblaze_tls_referenced_p (rtx x)
{
if (!TARGET_HAVE_TLS)
return false;
-
- return for_each_rtx (x, microblaze_tls_operand_p_1, NULL);
+ subrtx_iterator::array_type array;
+ FOR_EACH_SUBRTX (iter, array, x, ALL)
+{
+ const_rtx x = *iter;
+ if (GET_CODE (x) == SYMBOL_REF SYMBOL_REF_TLS_MODEL (x) != 0)
+ return true;
+ /* Don't recurse into UNSPEC_TLS looking for TLS symbols; these are
+TLS offsets, not real symbol references. */
+ if (GET_CODE (x) == UNSPEC XINT (x, 1) == UNSPEC_TLS)
+ iter.skip_subrtxes ();
+}
+ return false;
}
bool
--
Michael Eagerea...@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [PATCH] microblaze: microblaze.md: Use 'SI' instead of 'VOID' for operand 1 of 'call_value_intern'
On 10/26/14 03:36, Chen Gang wrote: On 10/22/2014 09:34 AM, Chen Gang wrote: Yes, if you want to test on a target, you will need a target. You can either have a simulator (see binutils and sim/* for an example of how to write one) or target hardware in some form. After tried 'sim', I found the root cause is microblaze sim does not support '--sysroot', which is the environments for shared libraries and system calls (need load microblaze kernel). - microblaze can successfully execute simple programs which has no glibc and no system call. - In upstream master branch of binutils, for microblaze sim, it has no related testsuite for sim in binutils, neither support '--sysroot', neither support function stack, startup parameters, and environments. - After hard code the default stack in sim, it can start the '-static' program with glibc, but stop at uname() which will use system call. So I want to consult: at present, can we let microblaze sim run 'normal' programs (have glibc, and use system call)? Microblaze-sim provides basic instruction set architecture and memory simulation. There is no operating system support. (It's also quite old. I'm not sure which version of the MB architecture it models, but it is not recent.) Microblaze-sim is not a full system simulator, like QEMU. To be able to run a program which requires glibc, you need to be able to boot a full Linux image on the simulator, which microblaze-sim cannot do. QEMU models an entire processor and can boot a Linux image. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [PATCH] microblaze: microblaze.md: Use VOID instead of SI to fix ((void (*)(void)) 0)() issue
On 09/25/14 07:03, Chen Gang wrote: 2014-09-25 Chen Ganggang.chen.5...@gmail.com gcc: * config/microblaze/microblaze.md (call_internal1): Use VOID instead of SI to fix ((void (*)(void)) 0)() issue gcc/testsuite/: 2014-09-28 Chen Gang gang.chen.5...@gmail.com * gcc.c-torture/compile/calls-void.c: New test. Committed revision 215684. Thanks for adding the test case. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [PATCH] microblaze: microblaze.md: Use VOID instead of SI to fix ((void (*)(void)) 0)() issue
On 09/25/14 07:03, Chen Gang wrote:
Need use VOID instead of SI, or when real VOIDmode comes, it does not
match SImode, so cause issue. This patch can fix this issue and pass
testsuite.
The related test code ('void' will cause CALL instead of SET):
typedef void (*T)(void);
f1 ()
{
((T) 0)();
}
The related error:
[root@localhost gcc]# ./cc1 /tmp/calls.c -o /tmp/1.s
f1
Analyzing compilation unit
Performing interprocedural optimizations
*free_lang_data visibility early_local_cleanups free-inline-summary
whole-program inlineAssembling functions:
f1
/tmp/calls.c: In function 'f1':
/tmp/calls.c:5:1: error: unrecognizable insn:
}
^
(call_insn 5 2 8 2 (parallel [
(call (mem:SI (const_int 0 [0]) [0 MEM[(void (*T29c)
(void))0B] S4 A32])
(const_int 24 [0x18]))
(clobber (reg:SI 15 r15))
]) /tmp/calls.c:4 -1
(nil)
(nil))
/tmp/calls.c:5:1: internal compiler error: in extract_insn, at recog.c:2204
0xb0e71b _fatal_insn(char const*, rtx_def const*, char const*, int, char
const*)
../../gcc/gcc/rtl-error.c:109
0xb0e75c _fatal_insn_not_found(rtx_def const*, char const*, int, char const*)
../../gcc/gcc/rtl-error.c:117
0xac552b extract_insn(rtx_def*)
../../gcc/gcc/recog.c:2204
0x8b919e instantiate_virtual_regs_in_insn
../../gcc/gcc/function.c:1614
0x8ba347 instantiate_virtual_regs
../../gcc/gcc/function.c:1934
0x8ba452 execute
../../gcc/gcc/function.c:1983
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See http://gcc.gnu.org/bugs.html for instructions.
Is this test case (or a similar one) in the gcc test suite?
If not, can you please add it to the test suite.
2014-09-25 Chen Gang gang.chen.5...@gmail.com
* config/microblaze/microblaze.md (call_internal1): Use VOID
instead of SI to fix ((void (*)(void)) 0)() issue
---
gcc/config/microblaze/microblaze.md | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gcc/config/microblaze/microblaze.md
b/gcc/config/microblaze/microblaze.md
index b971737..3b4faf4 100644
--- a/gcc/config/microblaze/microblaze.md
+++ b/gcc/config/microblaze/microblaze.md
@@ -2062,7 +2062,7 @@
(set_attr length4)])
(define_insn call_internal1
- [(call (mem (match_operand:SI 0 call_insn_simple_operand ri))
+ [(call (mem (match_operand:VOID 0 call_insn_simple_operand ri))
(match_operand:SI 1 i))
(clobber (reg:SI R_SR))]
I've verified that your patch does not cause any test suite regressions.
--
Michael Eagerea...@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [PATCH] microblaze: microblaze.md: Use VOID instead of SI to fix ((void (*)(void)) 0)() issue
On 09/25/14 07:03, Chen Gang wrote: Need use VOID instead of SI, or when real VOIDmode comes, it does not match SImode, so cause issue. This patch can fix this issue and pass testsuite. Did you forget to attach the patch? -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [PATCH] microblaze: microblaze.md: Use VOID instead of SI to fix ((void (*)(void)) 0)() issue
On 09/25/14 10:38, Michael Eager wrote: On 09/25/14 07:03, Chen Gang wrote: Need use VOID instead of SI, or when real VOIDmode comes, it does not match SImode, so cause issue. This patch can fix this issue and pass testsuite. Did you forget to attach the patch? Never mind. My eyes were playing tricks on me. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [PATCH] microblaze: microblaze.md: Use 'SI' instead of 'VOID' for operand 1 of 'call_value_intern'
On 09/24/14 07:31, Chen Gang wrote: Hello Michael: Firstly, thank you very much for always providing your aid to me for microblaze. At present, after try testsuite, the result is much better than my original trying, please help check the result: is it enough for our microblaze testsuite (can we say it pass checking)? Current result: # of expected passes65987 # of unexpected failures82 # of unexpected successes 1 # of expected failures 97 # of unresolved testcases 16378 # of unsupported tests 1810 This is good. Original result: # of expected passes 48408 # of unexpected failures 17253 # of unexpected successes 1 # of expected failures97 # of unresolved testcases 16570 # of unsupported tests1854 After check the current result log, I find many remote target test related sentences, do we have to process it? e.g. Download to microblaze-xilinx-gdb failed, couldn't execute rcp: no such file or directory. The test suite uses rcp to transfer files to or from the target, either to provide input to a test case or to check the output. Most Linux systems do not install rcp, since it is a security risk. And I guess, it is a glibc bug: which still add root directory (e.g. /upstream/release) in 'libc.so' when already has --with-sysroot for configure. Oh, sorry, glibc should also need --with-sysroot. I shall try it today, hope it will let all things OK. After add --with-sysroot for glibc, this issue is still existance. And I remove the redundant direcltory manually for libc.so and libpthread.so. If our microblaze testsuite is OK, I will skip this issue (since I have no enough time resource on glibc, at present). OK with me. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [PATCH] microblaze: microblaze.md: Use 'SI' instead of 'VOID' for operand 1 of 'call_value_intern'
On 09/24/14 09:23, Chen Gang wrote:
On 09/24/2014 11:37 PM, Mike Stump wrote:
On Sep 24, 2014, at 8:28 AM, Michael Eager ea...@eagerm.com wrote:
After check the current result log, I find many remote target test
related sentences, do we have to process it?
e.g. Download to microblaze-xilinx-gdb failed, couldn't execute rcp: no such
file or directory.
The test suite uses rcp to transfer files to or from the target,
either to provide input to a test case or to check the output.
Most Linux systems do not install rcp, since it is a security risk.
To clarify:
if {[board_info $desthost exists rcp_prog]} {
set RCP [board_info $desthost rcp_prog]
} else {
set RCP rcp
}
So, if you set rcp_prog to something else, you should be able to avoid rsh if
you want. Most people use ssh now-a-days. You will want it set up to not
require a password for testing.
OK, thank you for your information.
For one simple solving way under fedora: yum install rsh, and I will
get another issue:
Download to microblaze-xilinx-gdb failed, microblaze-xilinx-gdb: Unknown
host
So I guess the root cause is: I only use cross-compiling environments
under fedora x86_64, no any real or virtual target for test.
You can see the command which is getting the error by looking at gcc.log.
If the command is not displayed, add another -v (or two) to RUNTESTFLAGS.
If I recall correctly, DejaGNU is executing a command like rcp file
microblaze-xilinx-gdb.
This means that you need to have the name microblaze-xilinx-gdb resolved into
an IP address.
This can be done by adding it to /etc/hosts. Or you can specify the target IP
address
in the DejaGNU options. I don't recall which one off the top of my head.
--
Michael Eagerea...@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [PATCH] microblaze: microblaze.md: Use 'SI' instead of 'VOID' for operand 1 of 'call_value_intern'
On 09/21/14 21:10, Chen Gang wrote: On 9/22/14 2:09, Michael Eager wrote: Generally, you should use gcc to link programs, not ld. gcc is a driver which will select the appropriate libraries and support routines (such as crt0.o, which contains _start) and pass them to the linker. OK, thanks. When gcc, it misses the root directory for crt1.o and crtn.o: e.g. /lib/ld.so.1, crt1.o, crtn.o when gcc -v, but we need /upstream/ release/lib/ld.so.1, /upstream/lib/crt1.o, /upstream/libcrtn.o. You likely need to build mb-gcc with --sysroot=/upstream. How are you building gcc? What are your configuration options? -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [PATCH] microblaze: microblaze.md: Use 'SI' instead of 'VOID' for operand 1 of 'call_value_intern'
On 09/21/14 20:55, Chen Gang wrote: On 9/22/14 2:03, Michael Eager wrote: On 09/20/14 23:24, Chen Gang wrote: And it seems, we also need 'LinkScr.ld' for ldscript, could you share it to me, thanks. set_board_info ldscript -T/home/eager/Xilinx/dg/microblaze_0/LinkScr.ld Hi Chen -- The DejaGNU configuration I provided is for a bare-metal environment. If you are testing in a Linux environment, the tool chain you uses should provide a default linker script which matches your hardware's memory layout. You should not need to provide a separate linker script. OK, thanks, I shall try to find the default linker script for it. If you are running mb-gcc which generates executables which run on the target, you do not need to provide a linker script. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [PATCH] microblaze: microblaze.md: Use 'SI' instead of 'VOID' for operand 1 of 'call_value_intern'
On 09/20/14 23:24, Chen Gang wrote: And it seems, we also need 'LinkScr.ld' for ldscript, could you share it to me, thanks. set_board_info ldscript -T/home/eager/Xilinx/dg/microblaze_0/LinkScr.ld Hi Chen -- The DejaGNU configuration I provided is for a bare-metal environment. If you are testing in a Linux environment, the tool chain you uses should provide a default linker script which matches your hardware's memory layout. You should not need to provide a separate linker script. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [PATCH] microblaze: microblaze.md: Use 'SI' instead of 'VOID' for operand 1 of 'call_value_intern'
On 09/21/14 03:46, Chen Gang wrote: Excuse me, I want to consult one thing: I installed raw microblaze cross compiler, binutils and glibc into one directory (/upstream/release), and try to build a Hello world C program for microblaze under x86_64 host. I guess it is OK, but I am not quite sure about it (I use raw compiler, and have to point out /upstream/release/lib/ld.so.1 explicitly, and receive a warning related with 'ld'). Is it really OK? can I still use raw compiler for our testsuite? Is 'LinkScr.ld' for ldscript related with current case? [root@localhost test]# /upstream/release/bin/microblaze-gchen-linux-gcc -nostdinc -c -o test.o test.c [root@localhost test]# /upstream/release/bin/microblaze-gchen-linux-ld -o test test.o /upstream/release/lib/ld.so.1 -lc -L/upstream/release/lib /upstream/release/bin/microblaze-gchen-linux-ld: warning: cannot find entry symbol _start; defaulting to 1180 Generally, you should use gcc to link programs, not ld. gcc is a driver which will select the appropriate libraries and support routines (such as crt0.o, which contains _start) and pass them to the linker. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [PATCH] microblaze: microblaze.md: Use 'SI' instead of 'VOID' for operand 1 of 'call_value_intern'
On 09/20/14 08:52, Chen Gang wrote: Thank you very much for your attachments, it is very useful to me! I tried testsuite for microblaze cross target on x86_64 host, it says OK (echo $? == 0), but I am not quite sure about it (I still doubt that my configuration is incorrect), please help check, thanks. Welcome to the joys of DejaGNU. Configuration can be confusing. As you can see, the return code is not useful. dejagnu configuration: cp xmd.exp /usr/local/share/dejagnu/config/ cp microblaze-xilinx-gdb.exp /usr/local/share/dejagn/baseboards/ vi microblaze-xilinx-gdb.exp s/mc_gcc/microblaze\-gchen\-linux\-gcc/g gcc operation: ../gcc/configure --target=microblaze-gchen-linux --disable-nls --enable-languages=c --disable-threads --disable-shared \ --without-headers --disable-libssp --disable-libquadmath --disable-libgomp --disable-libatomic make make -k check-gcc RUNTESTFLAGS=--target_board=microblaze-xilinx-gdb/-mno-xl-soft-mul/-mxl-barrel-shift/-mcpu=v6.00.a Check whether these compiler options are being passed to mb-gcc. There is a line in my microblaze-xilinx-gdb.exp which sets CFLAGS: set_board_info cflags -mcpu=v4.00.b -mno-xl-soft-mul -mxl-barrel-shift This is likely overriding any options passed to runtest. Make sure that the options match the features of your target board. You might not need any options for your initial tests. Make sure that the correct flags are being passed to the linker. Add -v or -v -v to RUNTESTFLAGS so that the gcc.log file gives useful info. You might want to limit the number of tests run until you get problems worked out: make check-gcc RUNTESTFLAGS=execute.exp -v -v --target_board=microblaze-xilinx-gdb This will run only the gcc.c-torture/execute/execute.exp tests. gcc result: === gcc Summary === # of expected passes 48408 # of unexpected failures 17253 # of unexpected successes 1 # of expected failures97 # of unresolved testcases 16570 # of unsupported tests1854 /upstream/build-gcc/gcc/xgcc version 5.0.0 20140920 (experimental) (GCC) Look at gcc.sum and gcc.log to find out what is causing the large number of unexpected failures. A large number of unresolved test cases often means that the compiler returned an error. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [PATCH] microblaze: microblaze.md: Use 'SI' instead of 'VOID' for operand 1 of 'call_value_intern'
On 09/15/14 15:53, Chen Gang wrote:
On 09/15/2014 11:30 PM, Michael Eager wrote:
Configuring DejaGNU for cross-target testing requires creating a configuration
file describing the target board.
OK, thank you very much. And could you share your configuration file,
which I can reference to?
See attached.
--
Michael Eagerea...@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
# Xilinx remote for MicroBlaze using XMD
#load_generic_config monitor
load_generic_config xmd
# Identify multilib flags using libgloss
process_multilib_options
# The default compiler for this target.
set_board_info compiler mb-gcc
# We will be using the standard GDB remote protocol
set_board_info gdb_protocol remote
# Name of the computer whose socket will be used, if required.
#set_board_info sockethost gdb
set_board_info sockethost localhost
# Port ID to use for socket connection
set_board_info gdb,socketport 1234
# Port for target
#set_board_info netport gdb:1234
set_board_info netport localhost:1234
# Use techniques appropriate to a stub (don't do run command)
set_board_info use_gdb_stub 1
# This gdbserver can only run a process once per session.
set_board_info gdb,do_reload_on_run 1
# There's no support for argument-passing (yet).
set_board_info noargs 1
# Can't do input (or output) in the current gdbserver.
set_board_info gdb,noinferiorio 1
# Can't do singnals
set_board_info gdb,nosignals 1
# Can't do hardware watchpoints, in general
set_board_info gdb,no_hardware_watchpoints 1
#set_board_info cflags [newlib_include_flags] [libgloss_include_flags]
set_board_info cflags -mcpu=v4.00.b -mno-xl-soft-mul -mxl-barrel-shift
set_board_info addl_link_flags -L /home/eager/Xilinx/dg/microblaze_0/lib
-Wl,-defsym -Wl,_HEAP_SIZE=0x100 -Wl,-defsym -Wl,_STACK_SIZE=0x8
set_board_info ldscript -T/home/eager/Xilinx/dg/microblaze_0/LinkScr.ld
set_board_info ldflags [libgloss_link_flags] [newlib_link_flags] [board_info
$board addl_link_flags]
# Use remote protocol to XMD server
load_config monitor.exp
proc set_host_info { entry value } {
global target_info board_info
verbose set_host_info $entry $value 3
set machine host
if [info exists target_info($machine,name)] {
set machine $target_info($machine,name)
}
set board_info($machine,$entry) $value
}
proc gdb_target_exec { } {
send_gdb cont
#gdb_test target exec No executable file now. .*Kill it.*y or n.*
y
}
proc remote_reboot { host } {
}
Re: [PATCH] microblaze: microblaze.md: Use 'SI' instead of 'VOID' for operand 1 of 'call_value_intern'
On 09/14/14 21:19, Chen Gang wrote: On 9/15/14 11:03, Michael Eager wrote: On 09/14/14 00:51, Chen Gang wrote: Hello maintainers: I also find some warnings during compiling microblaze, I also shall try to fix them, but excuse me, I am not quite familiar the testsuite for microblaze, could you provide any related information for it? Hi Chen -- This is the gcc DejaGNU test suite. You can find info about DejaGNU at http://www.gnu.org/software/dejagnu. There is also info about testing GCC here: https://gcc.gnu.org/wiki/Testing_GCC OK, thanks. I finished about x86_64 testsuite, and also tried microblaze testsuite under x86_64 machine, but failed. Do I need any additional information for microblaze testsuite? I run tests using these options: -mno-xl-soft-mul -mxl-barrel-shift -mcpu=v6.00.a Configuring DejaGNU for cross-target testing requires creating a configuration file describing the target board. Rather than the standard make check-gcc described on the wiki page and elsewhere, I use a script which sets some configuration options and executes runtest directly. This uses a MicroBlaze processor simulator called vpexec which was included with an older version of Xilinx's EDK. Xilinx no longer supports vpexec. You can use a hardware target board to test microblaze-gcc, or a different simulator such as QEMU. OK, thanks, I shall try Qemu (it is the additional chance for me to familiar with Qemu). And do you mean I need try testsuite under the related microblaze host ( e.g. Qemu microblaze simulator)? After finish environments construction, I shall also try to test a patch for ((void (*)(void)) 0)() fixing. Thanks. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [PATCH] microblaze: microblaze.md: Use 'SI' instead of 'VOID' for operand 1 of 'call_value_intern'
On 09/14/14 00:51, Chen Gang wrote: Hello maintainers: I also find some warnings during compiling microblaze, I also shall try to fix them, but excuse me, I am not quite familiar the testsuite for microblaze, could you provide any related information for it? Hi Chen -- This is the gcc DejaGNU test suite. You can find info about DejaGNU at http://www.gnu.org/software/dejagnu. There is also info about testing GCC here: https://gcc.gnu.org/wiki/Testing_GCC Rather than the standard make check-gcc described on the wiki page and elsewhere, I use a script which sets some configuration options and executes runtest directly. This uses a MicroBlaze processor simulator called vpexec which was included with an older version of Xilinx's EDK. Xilinx no longer supports vpexec. You can use a hardware target board to test microblaze-gcc, or a different simulator such as QEMU. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [Patch, microblaze]: Add Init_priority support.
On 07/28/14 08:42, Ajit Kumar Agarwal wrote: Please find the following patch for init_priority support for microblaze. Testing Done : No regressions seen in gcc and g++ regressions testsuite. [Patch, microblaze]: Add Init_priority support. Added TARGET_ASM_CONSTRUCTOR and TARGET_ASM_DESTRUCTOR macros. These macros allows users to control the order of initialization of objects defined at namespace scope with the init_priority attribute by specifying a relative priority. ChangeLog: 2014-07-28 Ajit Agarwal ajit...@xilinx.com * config/microblaze/microblaze.c (microblaze_elf_asm_cdtor): New. (microblaze_elf_asm_constructor,microblaze_elf_asm_destructor): New. * config/microblaze/microblaze.h (TARGET_ASM_CONSTRUCTOR,TARGET_ASM_DESTRUCTOR): New Macros. Signed-off-by:Ajit Agarwal ajit...@xilinx.com. Committed revision 214110. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [PATCH] microblaze: microblaze.md: Use 'SI' instead of 'VOID' for operand 1 of 'call_value_intern'
On 07/06/14 03:26, Chen Gang wrote: * microblaze/mocroblaze.md (call_value_intern): Use 'SI' instead of 'VOID' for operand 1, just like 'call_internal1' has done. The related warning: ../../gcc/gcc/config/microblaze/microblaze.md:2172: warning: operand 1 missing mode? Signed-off-by: Chen Gang gang.chen.5...@gmail.com --- gcc/config/microblaze/microblaze.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/config/microblaze/microblaze.md b/gcc/config/microblaze/microblaze.md index 2bd5d72..9580221 100644 --- a/gcc/config/microblaze/microblaze.md +++ b/gcc/config/microblaze/microblaze.md @@ -2171,7 +2171,7 @@ (define_insn call_value_intern [(set (match_operand:VOID 0 register_operand =d) -(call (mem (match_operand:VOID 1 call_insn_operand ri)) +(call (mem (match_operand:SI 1 call_insn_operand ri)) (match_operand:SI 2 i))) (clobber (match_operand:SI 3 register_operand =d))] This patch causes a test suite regression: Executing on host: mb-gcc -fno-diagnostics-show-caret -fdiagnostics-color=never-O0 -w -c -mno-xl-soft-mul -mxl-barrel-shift -mcpu=v6.00.a -o calls.o testsuite/gcc.c-torture/compile/calls.c(timeout = 60) pid is 24832 -24832 testsuite/gcc.c-torture/compile/calls.c: In function 'f1': testsuite/gcc.c-torture/compile/calls.c:6:1: error: unrecognizable insn: (call_insn 5 2 8 2 (parallel [ (set (reg:SI 3 r3) (call (mem:SI (const_int 0 [0]) [0 MEM[(void * (*T29e) (void))0B] S4 A32]) (const_int 24 [0x18]))) (clobber (reg:SI 15 r15)) ]) testsuite/gcc.c-torture/compile/calls.c:5 -1 (nil) (nil)) testsuite/gcc.c-torture/compile/calls.c:6:1: internal compiler error: in extract_insn, at recog.c:2204 0x983018 _fatal_insn(char const*, rtx_def const*, char const*, int, char const*) /store/Xilinx/repo/fsf/gcc/gcc/rtl-error.c:109 0x983041 _fatal_insn_not_found(rtx_def const*, char const*, int, char const*) /store/Xilinx/repo/fsf/gcc/gcc/rtl-error.c:117 0x9539cd extract_insn(rtx_def*) /store/Xilinx/repo/fsf/gcc/gcc/recog.c:2204 0x7a5b59 instantiate_virtual_regs_in_insn /store/Xilinx/repo/fsf/gcc/gcc/function.c:1561 0x7aaa78 instantiate_virtual_regs /store/Xilinx/repo/fsf/gcc/gcc/function.c:1932 -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [PATCH] gcc/config/microblaze/microblaze.md: Remove redundant '@' to avoid compiling warning
On 07/05/14 20:59, Chen Gang wrote: * microblaze/microblaze.md: Remove redundant '@' to avoid compiling warning. The related warning: ../../gcc/gcc/config/microblaze/microblaze.md:516: '@' is redundant for output template with single alternative Signed-off-by: Chen Gang gang.chen.5...@gmail.com --- gcc/config/microblaze/microblaze.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/gcc/config/microblaze/microblaze.md b/gcc/config/microblaze/microblaze.md index 7945d96..2bd5d72 100644 --- a/gcc/config/microblaze/microblaze.md +++ b/gcc/config/microblaze/microblaze.md @@ -518,8 +518,7 @@ (minus:DI (match_operand:DI 1 register_operand d) (match_operand:DI 2 arith_operand32 d)))] - @ - rsub\t%L0,%L2,%L1\;rsubc\t%M0,%M2,%M1 + rsub\t%L0,%L2,%L1\;rsubc\t%M0,%M2,%M1 [(set_attr type darith) (set_attr mode DI) (set_attr length8)]) Committed revision 213913. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
