[S] Change in osmo-e1-hardware[master]: icE1usb fw: Workaround some apparent GCC aliasing bug ...
tnt has submitted this change. (
https://gerrit.osmocom.org/c/osmo-e1-hardware/+/36175?usp=email )
Change subject: icE1usb fw: Workaround some apparent GCC aliasing bug ...
..
icE1usb fw: Workaround some apparent GCC aliasing bug ...
If the code is built either :
* without -flto
* with -fno-strict-aliasing
* with that struct as 'static' (so it's not on the stack)
Then all works fine. But in the current situation (without the
patch), GCC seems to think there is some aliasing and just plain
removed the `notif.bits = 1` bit of code in that functions
(no warnings printed ...).
Putting the struct as 'static' is the least awful workaround.
I didn't bother reporting bug upstream, because I can't reproduce
on a small test case and I'm sure I'd just get yelled at and that
the compiler is right for some reason ...
Signed-off-by: Sylvain Munaut
Change-Id: Ie0a2ce337ce4a9c08c3d27acc4d922a3e5892840
---
M firmware/ice40-riscv/icE1usb/usb_gps.c
1 file changed, 28 insertions(+), 1 deletion(-)
Approvals:
tnt: Looks good to me, approved
Jenkins Builder: Verified
laforge: Looks good to me, but someone else must approve
manawyrm: Looks good to me, but someone else must approve
diff --git a/firmware/ice40-riscv/icE1usb/usb_gps.c
b/firmware/ice40-riscv/icE1usb/usb_gps.c
index f2b09b4..e219cf8 100644
--- a/firmware/ice40-riscv/icE1usb/usb_gps.c
+++ b/firmware/ice40-riscv/icE1usb/usb_gps.c
@@ -143,7 +143,8 @@
if ((ep_regs->bd[0].csr & USB_BD_STATE_MSK) != USB_BD_STATE_RDY_DATA)
{
/* Default request */
- struct usb_cdc_notif_serial_state notif = {
+ /* Put as static to work around gcc aliasing bug ... */
+ static struct usb_cdc_notif_serial_state notif = {
.hdr = {
.bmRequestType = USB_REQ_READ |
USB_REQ_TYPE_CLASS | USB_REQ_RCPT_INTF,
.bRequest = USB_NOTIF_CDC_SERIAL_STATE,
--
To view, visit https://gerrit.osmocom.org/c/osmo-e1-hardware/+/36175?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-e1-hardware
Gerrit-Branch: master
Gerrit-Change-Id: Ie0a2ce337ce4a9c08c3d27acc4d922a3e5892840
Gerrit-Change-Number: 36175
Gerrit-PatchSet: 1
Gerrit-Owner: tnt
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge
Gerrit-Reviewer: manawyrm
Gerrit-Reviewer: tnt
Gerrit-MessageType: merged
[S] Change in osmo-e1-hardware[master]: icE1usb fw: Workaround some apparent GCC aliasing bug ...
tnt has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-e1-hardware/+/36175?usp=email ) Change subject: icE1usb fw: Workaround some apparent GCC aliasing bug ... .. Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-e1-hardware/+/36175?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-e1-hardware Gerrit-Branch: master Gerrit-Change-Id: Ie0a2ce337ce4a9c08c3d27acc4d922a3e5892840 Gerrit-Change-Number: 36175 Gerrit-PatchSet: 1 Gerrit-Owner: tnt Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: manawyrm Gerrit-Reviewer: tnt Gerrit-Comment-Date: Wed, 06 Mar 2024 10:36:39 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[S] Change in osmo-e1-hardware[master]: icE1usb fw: Workaround some apparent GCC aliasing bug ...
Attention is currently required from: tnt. manawyrm has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-e1-hardware/+/36175?usp=email ) Change subject: icE1usb fw: Workaround some apparent GCC aliasing bug ... .. Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-e1-hardware/+/36175?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-e1-hardware Gerrit-Branch: master Gerrit-Change-Id: Ie0a2ce337ce4a9c08c3d27acc4d922a3e5892840 Gerrit-Change-Number: 36175 Gerrit-PatchSet: 1 Gerrit-Owner: tnt Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: manawyrm Gerrit-Attention: tnt Gerrit-Comment-Date: Wed, 06 Mar 2024 10:26:39 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[S] Change in osmo-e1-hardware[master]: icE1usb fw: Workaround some apparent GCC aliasing bug ...
Attention is currently required from: tnt. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-e1-hardware/+/36175?usp=email ) Change subject: icE1usb fw: Workaround some apparent GCC aliasing bug ... .. Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-e1-hardware/+/36175?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-e1-hardware Gerrit-Branch: master Gerrit-Change-Id: Ie0a2ce337ce4a9c08c3d27acc4d922a3e5892840 Gerrit-Change-Number: 36175 Gerrit-PatchSet: 1 Gerrit-Owner: tnt Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Attention: tnt Gerrit-Comment-Date: Wed, 06 Mar 2024 10:26:19 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[S] Change in osmo-e1-hardware[master]: icE1usb fw: Workaround some apparent GCC aliasing bug ...
tnt has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-e1-hardware/+/36175?usp=email )
Change subject: icE1usb fw: Workaround some apparent GCC aliasing bug ...
..
icE1usb fw: Workaround some apparent GCC aliasing bug ...
If the code is built either :
* without -flto
* with -fno-strict-aliasing
* with that struct as 'static' (so it's not on the stack)
Then all works fine. But in the current situation (without the
patch), GCC seems to think there is some aliasing and just plain
removed the `notif.bits = 1` bit of code in that functions
(no warnings printed ...).
Putting the struct as 'static' is the least awful workaround.
I didn't bother reporting bug upstream, because I can't reproduce
on a small test case and I'm sure I'd just get yelled at and that
the compiler is right for some reason ...
Signed-off-by: Sylvain Munaut
Change-Id: Ie0a2ce337ce4a9c08c3d27acc4d922a3e5892840
---
M firmware/ice40-riscv/icE1usb/usb_gps.c
1 file changed, 28 insertions(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-e1-hardware
refs/changes/75/36175/1
diff --git a/firmware/ice40-riscv/icE1usb/usb_gps.c
b/firmware/ice40-riscv/icE1usb/usb_gps.c
index f2b09b4..e219cf8 100644
--- a/firmware/ice40-riscv/icE1usb/usb_gps.c
+++ b/firmware/ice40-riscv/icE1usb/usb_gps.c
@@ -143,7 +143,8 @@
if ((ep_regs->bd[0].csr & USB_BD_STATE_MSK) != USB_BD_STATE_RDY_DATA)
{
/* Default request */
- struct usb_cdc_notif_serial_state notif = {
+ /* Put as static to work around gcc aliasing bug ... */
+ static struct usb_cdc_notif_serial_state notif = {
.hdr = {
.bmRequestType = USB_REQ_READ |
USB_REQ_TYPE_CLASS | USB_REQ_RCPT_INTF,
.bRequest = USB_NOTIF_CDC_SERIAL_STATE,
--
To view, visit https://gerrit.osmocom.org/c/osmo-e1-hardware/+/36175?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-e1-hardware
Gerrit-Branch: master
Gerrit-Change-Id: Ie0a2ce337ce4a9c08c3d27acc4d922a3e5892840
Gerrit-Change-Number: 36175
Gerrit-PatchSet: 1
Gerrit-Owner: tnt
Gerrit-MessageType: newchange
