CodeSamples: Fixes for ppc64

PATCH Applied Akira Yokosawa 2 participants 6 messages Jun 3, 2017 – Jun 4, 2017

Share & tools

Akira Yokosawa

author

2017-06-03 07:01 UTC #0 lore raw

cover CodeSamples: Fixes for ppc64

From 7c151a100ef7d6c30259a966c68c45421edd7707 Mon Sep 17 00:00:00 2001
From: Akira Yokosawa <[email protected]>
Date: Sat, 3 Jun 2017 13:32:49 +0900
Subject: [PATCH 0/2] CodeSamples: Fixes for ppc64

Hi Paul,

With the help of you, I now have an access to a ppc64 instance on OSU Open Source Lab.

I tried to build under CodeSamples/ there.
But I needed to fix dependency rules I updated recently. Also, script for ppc64 in Makefile failed to copy api-pthreads/api-gcc.h to api.h.
Patch 1 fixes them.

Patch 2 removes GCC flags specific to POWER5, and update links to pages with regard to optimization for POWER.

And there are a few things I want to confirm regarding CodeSamples.

(1) Is libucru a must now?

If it is the case, we should remove "else" blocks in Makefile which copu linux/list.h to api.h.

(2) Is little endian assuemed?

You suggested to choose ppc64le for the instance on OSU Open Source Lab. Is there any code that doesn't work on big-endian platform?

(3) Can we remove gprof-helper.c?

Although I fixed the entry in .gitignore the other day, gprof-helper.c lives in the repository from the beginning. Can we get rid of it?

Thanks, Akira

Signature · 11 lines

Akira Yokosawa (2):
  CodeSamples: Fixes for build on ppc64le
  CodeSamples: Remove cpu specific flag for ppc64

 CodeSamples/Makefile                 | 2 ++
 CodeSamples/arch-ppc64/Makefile.arch | 7 +++++--
 CodeSamples/depends.mk               | 2 ++
 3 files changed, 9 insertions(+), 2 deletions(-)

-- 
2.7.4

Akira Yokosawa

author

2017-06-03 07:03 UTC #1 lore raw

PATCH 1/2 CodeSamples: Fixes for build on ppc64le

From 9fdd8f22bc417c53b4467ccd997b9d2fb833832a Mon Sep 17 00:00:00 2001
From: Akira Yokosawa <[email protected]>
Date: Sat, 3 Jun 2017 02:23:20 +0000
Subject: [PATCH 1/2] CodeSamples: Fixes for build on ppc64le

Signed-off-by Akira Yokosawa <[email protected]>

---
 CodeSamples/Makefile   | 2 ++
 CodeSamples/depends.mk | 2 ++
 2 files changed, 4 insertions(+)

CodeSamples/Makefile +2 -0

index 4168220..e1593a7 100644
@@ -92,6 +92,8 @@ pthreads-ppc64:
 	echo "" >> api.h
 	cat api-pthreads/api-pthreads.h >> api.h
 	echo "" >> api.h
+	cat api-pthreads/api-gcc.h >> api.h
+	echo "" >> api.h
 	if test -f /usr/include/urcu-call-rcu.h -o -f /usr/local/include/urcu-call-rcu.h ; \
 	then \
 		echo "#define _LGPL_SOURCE" >> api.h; \

CodeSamples/depends.mk +2 -0

index 409eeee..f3a325d 100644
@@ -8,6 +8,8 @@ else ifeq ($(arch),x86_64)
 target := x86
 else ifeq ($(arch),ppc64)
 target := ppc64
+else ifeq ($(arch),ppc64le)
+target := ppc64
 else ifneq (,$(findstring arm,$(arch)))
 target := arm
 else

Signature · 1 line

2.7.4

Akira Yokosawa

author

2017-06-03 07:04 UTC #2 lore raw

PATCH 2/2 CodeSamples: Remove cpu specific flag for ppc64

In reply to Akira Yokosawa

From 7c151a100ef7d6c30259a966c68c45421edd7707 Mon Sep 17 00:00:00 2001
From: Akira Yokosawa <[email protected]>
Date: Sat, 3 Jun 2017 12:42:38 +0900
Subject: [PATCH 2/2] CodeSamples: Remove cpu specific flag for ppc64

Also update links to tuning options for POWER.

Signed-off-by Akira Yokosawa <[email protected]>

---
 CodeSamples/arch-ppc64/Makefile.arch | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

CodeSamples/arch-ppc64/Makefile.arch +5 -2

index 90a8ecf..cae45e3 100644
@@ -1,2 +1,5 @@
-# http://www.ibm.com/developerworks/wikis/display/hpccentral/Tuning+options+to+consider+with+gcc#Tuningoptionstoconsiderwithgcc-mcpuandmtune
-GCC_ARGS=-g -Wall -O3 -m64 -mcpu=power5 -mtune=power5
+# You might want to add a flag such as -mcpu=power8.
+# See e.g.:
+# https://www.ibm.com/developerworks/community/wikis/form/anonymous/api/wiki/366c0058-50f7-4c6d-9505-9d8af81d40f1/page/56044ce1-e853-47f2-a038-c7c8ea8bef23/media?convertTo=html (power5 & power6 era)
+# https://www.ibm.com/developerworks/community/wikis/home?lang=en#!/wiki/W51a7ffcf4dfd_4b40_9d82_446ebc23c550/page/New%20to%20PowerLinux%20tuning%20and%20optimizations?section=gcc (not so up to date)
+GCC_ARGS=-g -Wall -O3 -m64

Signature · 1 line

2.7.4

Paul E. McKenney

Applied

2017-06-03 20:20 UTC #3 lore raw

In reply to Akira Yokosawa

Quoted 19 lines · Akira Yokosawa

Quoted 1 line
From 7c151a100ef7d6c30259a966c68c45421edd7707 Mon Sep 17 00:00:00 2001
From: Akira Yokosawa <[email protected]>
Date: Sat, 3 Jun 2017 13:32:49 +0900
Subject: [PATCH 0/2] CodeSamples: Fixes for ppc64
Hi Paul,
With the help of you, I now have an access to a ppc64 instance on OSU Open Source Lab.
I tried to build under CodeSamples/ there.
But I needed to fix dependency rules I updated recently. Also, script for ppc64 in Makefile failed to copy api-pthreads/api-gcc.h to api.h.
Patch 1 fixes them.
Patch 2 removes GCC flags specific to POWER5, and update links to pages with regard to optimization for POWER.

Quoted 6 lines

And there are a few things I want to confirm regarding CodeSamples.
(1) Is libucru a must now?
If it is the case, we should remove "else" blocks in Makefile which copu linux/list.h to api.h.

These were put in place because old versions of liburcu didn't support lists. I would guess that there still are some distros using those old versions, but if not, then you are right that there would be no point in keeping the "else" blocks.

Quoted 4 lines

(2) Is little endian assuemed?
You suggested to choose ppc64le for the instance on OSU Open Source Lab. Is there any code that doesn't work on big-endian platform?

Things should work on both big-endian and on little-endian. But Power is slowly shifting from big-endian to little-endian, so I figured we should look to the future rather than the past.

Hmmm... Do you have access to some big-endian Linux system? Maybe a SPARC or some such?

Quoted 4 lines

(3) Can we remove gprof-helper.c?
Although I fixed the entry in .gitignore the other day, gprof-helper.c lives in the repository from the beginning. Can we get rid of it?

It was necessary long ago, but it has been one long time since I used gprof. I tend to use perf instead. So no objection here to removing it.

Quoted 5 lines

Thanks, Akira

Signature · 3 lines

Akira Yokosawa (2):
  CodeSamples: Fixes for build on ppc64le
  CodeSamples: Remove cpu specific flag for ppc64

Queued and pushed, thank you!

Thanx, Paul

Quoted 8 lines

 CodeSamples/Makefile                 | 2 ++
 CodeSamples/arch-ppc64/Makefile.arch | 7 +++++--
 CodeSamples/depends.mk               | 2 ++
 3 files changed, 9 insertions(+), 2 deletions(-)

Signature · 1 line

2.7.4

Akira Yokosawa

author

2017-06-03 23:32 UTC #4 lore raw

Quoted 24 lines · Paul E. McKenney

Quoted 23 lines · Akira Yokosawa
Quoted 1 line
From 7c151a100ef7d6c30259a966c68c45421edd7707 Mon Sep 17 00:00:00 2001
From: Akira Yokosawa <[email protected]>
Date: Sat, 3 Jun 2017 13:32:49 +0900
Subject: [PATCH 0/2] CodeSamples: Fixes for ppc64
Hi Paul,
With the help of you, I now have an access to a ppc64 instance on OSU Open Source Lab.
I tried to build under CodeSamples/ there.
But I needed to fix dependency rules I updated recently. Also, script for ppc64 in Makefile failed to copy api-pthreads/api-gcc.h to api.h.
Patch 1 fixes them.
Patch 2 removes GCC flags specific to POWER5, and update links to pages with regard to optimization for POWER.
And there are a few things I want to confirm regarding CodeSamples.
(1) Is libucru a must now?

Quoted 8 lines

Quoted 2 lines
If it is the case, we should remove "else" blocks in Makefile which copu linux/list.h to api.h.
These were put in place because old versions of liburcu didn't support lists. I would guess that there still are some distros using those old versions, but if not, then you are right that there would be no point in keeping the "else" blocks.

I asked this because without liburcu installed, build fails with errors and warnings saying:

Quoted 54 lines

cc -g -O3 -Wall -g -o lockdeq -DTEST lockdeq.c -lpthread lockdeq.c:70:23: error: field 'chain' has incomplete type

  struct cds_list_head chain;
                       ^
lockdeq.c: In function 'init_pdeq':
lockdeq.c:103:3: warning: implicit declaration of function 'CDS_INIT_LIST_HEAD' [-Wimplicit-function-declaration]
   CDS_INIT_LIST_HEAD(&d->bkt[i].chain);
   ^
lockdeq.c: In function 'pdeq_pop_l':
lockdeq.c:117:6: warning: implicit declaration of function 'cds_list_empty' [-Wimplicit-function-declaration]
  if (cds_list_empty(&p->chain))
      ^
lockdeq.c:121:3: warning: implicit declaration of function 'cds_list_del' [-Wimplicit-function-declaration]
   cds_list_del(e);
   ^

lockdeq.c: In function 'pdeq_push_l':
lockdeq.c:162:2: warning: implicit declaration of function 'cds_list_add_tail' [-Wimplicit-function-declaration]
cds_list_add_tail(e, &p->chain);
^
lockdeq.c: In function 'pdeq_push_r':
lockdeq.c:177:2: warning: implicit declaration of function 'cds_list_add' [-Wimplicit-function-declaration]
cds_list_add(e, &p->chain);
^
In file included from lockdeq.c:184:0:
deqtorture.h: At top level:
deqtorture.h:24:23: error: field 'l' has incomplete type

  struct cds_list_head l;
                       ^
deqtorture.h: In function 'concurrent_ppop':
deqtorture.h:110:8: warning: implicit declaration of function 'cds_list_entry' [-Wimplicit-function-declaration]
   p1 = cds_list_entry(p0, struct deq_elem, l);
        ^
deqtorture.h:110:27: error: expected expression before 'struct'
   p1 = cds_list_entry(p0, struct deq_elem, l);
                           ^
deqtorture.h: In function 'simple_deq_perf':
deqtorture.h:359:23: error: storage size of 'd' isn't known
  struct cds_list_head d;
                       ^
deqtorture.h:359:23: warning: unused variable 'd' [-Wunused-variable]
deqtorture.h: In function 'getdata':
deqtorture.h:477:27: error: expected expression before 'struct'
  return cds_list_entry(p, struct deq_elem, l)->data;
                           ^
deqtorture.h: In function 'main':
deqtorture.h:483:26: warning: variable 'e3' set but not used [-Wunused-but-set-variable]
  struct deq_elem e1, e2, e3;
                          ^
deqtorture.h:483:22: warning: variable 'e2' set but not used [-Wunused-but-set-variable]
  struct deq_elem e1, e2, e3;
                      ^
deqtorture.h:483:18: warning: variable 'e1' set but not used [-Wunused-but-set-variable]
  struct deq_elem e1, e2, e3;
                  ^

Quoted 6 lines

deqtorture.h: In function 'getdata':
deqtorture.h:478:1: warning: control reaches end of non-void function [-Wreturn-type]
}
^
Makefile:35: recipe for target 'lockdeq' failed
make[1]: *** [lockdeq] Error 1

Can you look into this?
I suppose linux/list.h needs some update.

Quoted 11 lines

Quoted 4 lines
(2) Is little endian assuemed?
You suggested to choose ppc64le for the instance on OSU Open Source Lab. Is there any code that doesn't work on big-endian platform?
Things should work on both big-endian and on little-endian. But Power is slowly shifting from big-endian to little-endian, so I figured we should look to the future rather than the past.
Hmmm... Do you have access to some big-endian Linux system? Maybe a SPARC or some such?

No.
I asked because if it is known to not work on big-endian, I thought "make" should spit some warning. And I didn't know Ubuntu's POWER support is only for ppc64le... So, never mind.

Quoted 8 lines

Quoted 5 lines
(3) Can we remove gprof-helper.c?
Although I fixed the entry in .gitignore the other day, gprof-helper.c lives in the repository from the beginning. Can we get rid of it?
It was necessary long ago, but it has been one long time since I used gprof. I tend to use perf instead. So no objection here to removing it.

I see. I'll submit a patch to do so.

Quoted 8 lines

Quoted 6 lines
Thanks, Akira
Signature · 3 lines
Akira Yokosawa (2):
  CodeSamples: Fixes for build on ppc64le
  CodeSamples: Remove cpu specific flag for ppc64
Queued and pushed, thank you!

BTW, I can create an account for you on the OSUOSL instance (Ubuntu 16.04). I believe you have accesses to ppc64, but having another would do no harm.
Let me know your SSH public key if you'd like to.

Thanks, Akira

Quoted 13 lines

Thanx, Paul

Quoted 8 lines

 CodeSamples/Makefile                 | 2 ++
 CodeSamples/arch-ppc64/Makefile.arch | 7 +++++--
 CodeSamples/depends.mk               | 2 ++
 3 files changed, 9 insertions(+), 2 deletions(-)

Signature · 1 line

2.7.4

Paul E. McKenney

2017-06-04 21:58 UTC #5 lore raw

Quoted 97 lines · Akira Yokosawa

Quoted 24 lines · Paul E. McKenney
Quoted 23 lines · Akira Yokosawa
Quoted 1 line
From 7c151a100ef7d6c30259a966c68c45421edd7707 Mon Sep 17 00:00:00 2001
From: Akira Yokosawa <[email protected]>
Date: Sat, 3 Jun 2017 13:32:49 +0900
Subject: [PATCH 0/2] CodeSamples: Fixes for ppc64
Hi Paul,
With the help of you, I now have an access to a ppc64 instance on OSU Open Source Lab.
I tried to build under CodeSamples/ there.
But I needed to fix dependency rules I updated recently. Also, script for ppc64 in Makefile failed to copy api-pthreads/api-gcc.h to api.h.
Patch 1 fixes them.
Patch 2 removes GCC flags specific to POWER5, and update links to pages with regard to optimization for POWER.
And there are a few things I want to confirm regarding CodeSamples.
(1) Is libucru a must now?
Quoted 7 lines
Quoted 2 lines
If it is the case, we should remove "else" blocks in Makefile which copu linux/list.h to api.h.
These were put in place because old versions of liburcu didn't support lists. I would guess that there still are some distros using those old versions, but if not, then you are right that there would be no point in keeping the "else" blocks.
I asked this because without liburcu installed, build fails with errors and warnings saying:
Quoted 54 lines
cc -g -O3 -Wall -g -o lockdeq -DTEST lockdeq.c -lpthread lockdeq.c:70:23: error: field 'chain' has incomplete type
  struct cds_list_head chain;
                       ^
lockdeq.c: In function 'init_pdeq':
lockdeq.c:103:3: warning: implicit declaration of function 'CDS_INIT_LIST_HEAD' [-Wimplicit-function-declaration]
   CDS_INIT_LIST_HEAD(&d->bkt[i].chain);
   ^
lockdeq.c: In function 'pdeq_pop_l':
lockdeq.c:117:6: warning: implicit declaration of function 'cds_list_empty' [-Wimplicit-function-declaration]
  if (cds_list_empty(&p->chain))
      ^
lockdeq.c:121:3: warning: implicit declaration of function 'cds_list_del' [-Wimplicit-function-declaration]
   cds_list_del(e);
   ^
lockdeq.c: In function 'pdeq_push_l':
lockdeq.c:162:2: warning: implicit declaration of function 'cds_list_add_tail' [-Wimplicit-function-declaration]
cds_list_add_tail(e, &p->chain);
^
lockdeq.c: In function 'pdeq_push_r':
lockdeq.c:177:2: warning: implicit declaration of function 'cds_list_add' [-Wimplicit-function-declaration]
cds_list_add(e, &p->chain);
^
In file included from lockdeq.c:184:0:
deqtorture.h: At top level:
deqtorture.h:24:23: error: field 'l' has incomplete type
  struct cds_list_head l;
                       ^
deqtorture.h: In function 'concurrent_ppop':
deqtorture.h:110:8: warning: implicit declaration of function 'cds_list_entry' [-Wimplicit-function-declaration]
   p1 = cds_list_entry(p0, struct deq_elem, l);
        ^
deqtorture.h:110:27: error: expected expression before 'struct'
   p1 = cds_list_entry(p0, struct deq_elem, l);
                           ^
deqtorture.h: In function 'simple_deq_perf':
deqtorture.h:359:23: error: storage size of 'd' isn't known
  struct cds_list_head d;
                       ^
deqtorture.h:359:23: warning: unused variable 'd' [-Wunused-variable]
deqtorture.h: In function 'getdata':
deqtorture.h:477:27: error: expected expression before 'struct'
  return cds_list_entry(p, struct deq_elem, l)->data;
                           ^
deqtorture.h: In function 'main':
deqtorture.h:483:26: warning: variable 'e3' set but not used [-Wunused-but-set-variable]
  struct deq_elem e1, e2, e3;
                          ^
deqtorture.h:483:22: warning: variable 'e2' set but not used [-Wunused-but-set-variable]
  struct deq_elem e1, e2, e3;
                      ^
deqtorture.h:483:18: warning: variable 'e1' set but not used [-Wunused-but-set-variable]
  struct deq_elem e1, e2, e3;
                  ^
Quoted 6 lines
deqtorture.h: In function 'getdata':
deqtorture.h:478:1: warning: control reaches end of non-void function [-Wreturn-type]
}
^
Makefile:35: recipe for target 'lockdeq' failed
make[1]: *** [lockdeq] Error 1

Quoted 2 lines

Can you look into this?
I suppose linux/list.h needs some update.

At this point, I am happy to have the dependency on a recent version of liburcu. It is readily available, and it does not make much sense to try to replicate its code within perfbook. ;-)

Quoted 9 lines

Quoted 9 lines
Quoted 4 lines
(2) Is little endian assuemed?
You suggested to choose ppc64le for the instance on OSU Open Source Lab. Is there any code that doesn't work on big-endian platform?
Things should work on both big-endian and on little-endian. But Power is slowly shifting from big-endian to little-endian, so I figured we should look to the future rather than the past.

Quoted 7 lines

Quoted 2 lines
Hmmm... Do you have access to some big-endian Linux system? Maybe a SPARC or some such?
No.
I asked because if it is known to not work on big-endian, I thought "make" should spit some warning. And I didn't know Ubuntu's POWER support is only for ppc64le... So, never mind.

Fair enough!

Quoted 9 lines

Quoted 7 lines
Quoted 4 lines
(3) Can we remove gprof-helper.c?
Although I fixed the entry in .gitignore the other day, gprof-helper.c lives in the repository from the beginning. Can we get rid of it?
It was necessary long ago, but it has been one long time since I used gprof. I tend to use perf instead. So no objection here to removing it.
I see. I'll submit a patch to do so.

Looking forward to it!

Quoted 6 lines

Quoted 6 lines
Quoted 5 lines
Thanks, Akira
Signature · 3 lines
Akira Yokosawa (2):
  CodeSamples: Fixes for build on ppc64le
  CodeSamples: Remove cpu specific flag for ppc64

Quoted 6 lines

Quoted 1 line
Queued and pushed, thank you!
BTW, I can create an account for you on the OSUOSL instance (Ubuntu 16.04). I believe you have accesses to ppc64, but having another would do no harm.
Let me know your SSH public key if you'd like to.

No need! I do have access, and it won't look good for me to be using your machine. ;-)

The main reason I wanted you to have your own access was to make it so that you don't have to wait for me to get around to running tests on your code.

Thanx, Paul

Quoted 16 lines

Thanks, Akira

Quoted 13 lines

Thanx, Paul

Quoted 8 lines

 CodeSamples/Makefile                 | 2 ++
 CodeSamples/arch-ppc64/Makefile.arch | 7 +++++--
 CodeSamples/depends.mk               | 2 ++
 3 files changed, 9 insertions(+), 2 deletions(-)

Signature · 1 line

2.7.4