bigk2000 发表于 2022-1-20 15:16:28

大家来看看,这个是代码不严谨,还是编译器疏漏

问题:
编译器应该用2处的uwElementCnt的值作为返回条件,
还是应该使用1处读取的值作为返回条件,
还是代码书写不严谨

tang_qianfeng 发表于 2022-1-20 15:33:22

本帖最后由 tang_qianfeng 于 2022-1-20 15:35 编辑

我觉得应该是2处的,是不是编译器的优化等级设置的不同?1处和2处值会不同吗? 有中断修改它?还是这是端口地址?

bigk2000 发表于 2022-1-20 15:38:04

tang_qianfeng 发表于 2022-1-20 15:33
我觉得应该是2处的,是不是编译器的优化等级设置的不同?1处和2处值会不同吗? 有中断修改它?还是这是端口 ...

优化最高,size模式,
oVoiceIdFifo.Read(puwID)里修改了uwElementCnt的值,其他地方不修改

bigk2000 发表于 2022-1-20 15:41:16

我的代码:
if(oVoiceIdFifo.GetUsedLength() > 0)
{
    oVoiceIdFifo.Read(puwID);
    return 1;
}


而编译器帮我改成了:
if(oVoiceIdFifo.GetUsedLength() > 0)
{
    oVoiceIdFifo.Read(puwID);
   
    if(oVoiceIdFifo.GetUsedLength() > 0)
      return 1;
}

bigk2000 发表于 2022-1-20 15:42:58

此类优化问题,如果不看汇编,是找不出问题在哪的

xmlbb 发表于 2022-1-20 15:45:25

代码是不规范,既然里面有判断了,外面就不用判断了,直接读就好了。
不过编译器貌似也有问题,如果没有开优化第二次读应该放read里面。

初音之恋 发表于 2022-1-20 15:50:22

bigk2000 发表于 2022-1-20 15:41
我的代码:




逻辑没问题 负优化...还不如直接return oVoiceIdFifo.Read(puwID);

wye11083 发表于 2022-1-20 16:09:09

初音之恋 发表于 2022-1-20 15:50
逻辑没问题 负优化...还不如直接return oVoiceIdFifo.Read(puwID);

逻辑显然是有问题的。。万一这个线程被挂起了,等恢复之后发现值变了。。估计是编译器的锅。所以gcc一定得用稳定版。

xinyou 发表于 2022-1-20 16:23:04

wye11083 发表于 2022-1-20 16:09
逻辑显然是有问题的。。万一这个线程被挂起了,等恢复之后发现值变了。。估计是编译器的锅。所以gcc一定 ...

仅从lz贴出来的代码,逻辑应是没有问题的。但是 uwElementCnt 没有加volatile,在其他的地方修改了,是另外的问题。

tang_qianfeng 发表于 2022-1-20 16:31:36

bigk2000 发表于 2022-1-20 15:38
优化最高,size模式,
oVoiceIdFifo.Read(puwID)里修改了uwElementCnt的值,其他地方不修改 ...

优化最高的话,这个不好评估了,有的代码都可能会重用。。。可能不同版本的iar优化策略不同

lyl1070 发表于 2022-1-20 17:27:55

肯定是你代码问题,我的代码都是一直开最高优化。

只有一次,在KEIL中出过问题,有个整型数据整体提升的打开了还是怎么样的。

2nd 发表于 2022-1-20 19:02:38

编译的明显有问题,而且楼主已经说了V8.4编译正确

dykwai1 发表于 2022-1-21 08:12:41

如果不是必要(存储空间、执行效率。。。),编译器尽量不要开级别太高优化
IAR keil 。。。开级别太高优化都易出类似问题

zhonghua_li 发表于 2022-1-21 09:07:06

都没问题,这是一种等效优化,如果不接受,可以禁用该优化。

security 发表于 2022-1-21 09:18:04

看起来是编译器问题,编译器也是人写的,有 bug 是正常的,只能是绕过去或者可以去提交问题。

gonboy 发表于 2022-1-21 11:08:33

本帖最后由 gonboy 于 2022-1-21 11:10 编辑

所谓的编译器Bug, 99.9999% 。都是代码编写不严谨,进行高度优化导致的。
仔细检查下代码吧,是否对每个优化的选项 都十分的了解。

不了解,就不要随便开高级优化。

-----------------------------------
这个问题,测试方法。
1. 无优化是否有问题?
2. 高级优化,逐步开启每个选项,看是哪个优化造成的。

如果debug,无优化 就有问题,就需要仔细分析

bigk2000 发表于 2022-1-21 11:11:04

本帖最后由 bigk2000 于 2022-1-21 11:18 编辑

这是IAR 8.4的汇编
这个编译就忠于C代码
且这个代码量要小于9.2的,速度上也似乎优于9.2

bigk2000 发表于 2022-1-21 11:36:33

本帖最后由 bigk2000 于 2022-1-21 11:41 编辑

gonboy 发表于 2022-1-21 11:08
所谓的编译器Bug, 99.9999% 。都是代码编写不严谨,进行高度优化导致的。
仔细检查下代码吧,是否对每个优 ...

我感觉这次是编译器的问题,
在2处又读一次已经改变了C代码的表述
编译器假设了在1处的值和2处的值是一样的,这显然在逻辑上是错误的
无论加不加volatile,编译器都不应该使用2处的值作为返回条件
而IAR 8.4的编译结果则完全忠于C代码表述

security 发表于 2022-1-21 11:43:43

bigk2000 发表于 2022-1-21 11:36
我感觉这次是编译器的问题,
在2处又读一次已经改变了C代码的表述
编译器假设了在1处的值和2处的值是一样 ...

是的。
不要怀疑你自己:你已经看得很深了,问题原因没那么浅。

gonboy 发表于 2022-1-21 12:01:48

bigk2000 发表于 2022-1-21 11:36
我感觉这次是编译器的问题,
在2处又读一次已经改变了C代码的表述
编译器假设了在1处的值和2处的值是一样 ...

方便的话,你可以写个类似问题的简单Demo ,发上来,大家看下。
这样 大家都是在猜
-------------------------------
debug 模式,是否是Ok的????

flamma 发表于 2022-1-21 12:06:03

这个看上去是编译器的问题。

bigk2000 发表于 2022-1-21 13:48:24

gonboy 发表于 2022-1-21 12:01
方便的话,你可以写个类似问题的简单Demo ,发上来,大家看下。
这样 大家都是在猜
--------------------- ...

就是debug模式出现的bug
做了个工程,无法复现bug

bigk2000 发表于 2022-1-21 15:16:11

本帖最后由 bigk2000 于 2022-1-21 15:17 编辑

gonboy 发表于 2022-1-21 12:01
方便的话,你可以写个类似问题的简单Demo ,发上来,大家看下。
这样 大家都是在猜
--------------------- ...

复现bug的工程,使用IAR 9.20打开,直接仿真运行就行

security 发表于 2022-1-21 15:49:13

bigk2000 发表于 2022-1-21 15:16
复现bug的工程,使用IAR 9.20打开,直接仿真运行就行

我 9.10 也是一样有问题。
仅仅把 Fuction inlining 禁止掉,就可以绕过去了。

MasterPhi 发表于 2022-1-22 05:29:45

直接和IAR技术支持反应,把工程发过去。
支持还是很不错的,每次都是一天内给我回复,就是实际修复bug得等久一点{:lol:}

bigk2000 发表于 2022-1-22 10:21:39

MasterPhi 发表于 2022-1-22 05:29
直接和IAR技术支持反应,把工程发过去。
支持还是很不错的,每次都是一天内给我回复,就是实际修复bug得等 ...

IAR技术支持填表要正版序列号,否则无法提交

gonboy 发表于 2022-1-22 11:46:52

刚测试了下,确认会出现此问题。

1.Fuction inlining 禁止掉 。 可以解决此问题。
2.测了下,如果不用模板类。也不会出现此类 bug。
3. 采用 高级优化 平衡/速度, 也可以解决此问题。


MasterPhi 发表于 2022-1-22 17:01:04

IAR技术支持填表要正版序列号,否则无法提交
有空我帮你提交下

bigk2000 发表于 2022-1-22 18:24:52

MasterPhi 发表于 2022-1-22 17:01
有空我帮你提交下

多谢!!!

bigk2000 发表于 2022-1-22 18:26:15

gonboy 发表于 2022-1-22 11:46
刚测试了下,确认会出现此问题。

1.Fuction inlining 禁止掉 。 可以解决此问题。


已知有bug,可以使用这些对策
不知有bug,就掉坑里了

MasterPhi 发表于 2022-1-24 19:12:29

好像是已知bug, 9.20.4会修复,他再找开发团队确认一下

bigk2000 发表于 2022-1-24 19:36:28

MasterPhi 发表于 2022-1-24 19:12
好像是已知bug, 9.20.4会修复,他再找开发团队确认一下

答复挺快的

zhongjp85 发表于 2022-1-27 09:54:57

MasterPhi 发表于 2022-1-24 19:12
好像是已知bug, 9.20.4会修复,他再找开发团队确认一下

这速度~

SCREA 发表于 2022-1-27 20:11:55

MasterPhi 发表于 2022-1-24 19:12
好像是已知bug, 9.20.4会修复,他再找开发团队确认一下

IAR 多少钱买的?

zchong 发表于 2022-1-28 07:36:06

目前是什么版本?

MasterPhi 发表于 2022-1-29 00:19:48

今天技术支持给我了个超长的回复, 大意如下:

[*] 多个地方会被修改的变量一定要加volatile
[*] 高优化下面这个函数被inline了,因为没有volatile所以变量可以读多次,这里读了两次是因为有些情况不用jump (具体看他回复)
[*] 加了个volatile以后结果就符合预期了


===
The discussion which follows points out effects of volatile, and what the compiler can decide to do when volatile isn't used.

As the 'uwElementCnt' can also be changed from interrupts, then I understand the user concern about multiple reads.

My 1st advice to the user:
All variables that can be changed from interrupts and are used in other parts of the application should be given the keyword 'volatile'.
- The compiler 'reads' the volatile keyword as: '...By declaring an object volatile, the compiler is informed that the value of the object can change beyond the compiler’s control. The compiler must also assume that any accesses can have side effects—therefore all accesses to the volatile object must be preserved...' (extract from the Development Guide.
- There are three main reasons for declaring an object volatile:
--- Shared access—the object is shared between several tasks in a multitasking environment
--- Trigger access—as for a memory-mapped SFR where the fact that an access occurs has an effect
--- Modified access—where the contents of the object can change in ways not known to the compiler.

Now back to the user request.

When (as the source was earlier written) the compiler applied the various optimizations (that are in use on high level), the 'function inlining' was performed, so the instructions (for source line 106) looks like...
106 if(oVoiceIdFifo.GetUsedLength() > 0)
\ 0x4 0x6AE0 LDR R0,
\ 0x6 0xB118 CBZ.N R0,??GetVoiceID_0
...so the value loaded to R0 (in other words the value of 'uwElementCnt') is in the CBZ.N compared to 'zero', and if zero the instructions for the source line 108 are note executed as exection goes to the label...
??GetVoiceID_0
...where that label is part of what is generated for source line 112...
112 return 1;
\ ??GetVoiceID_0: (+1)
\ 0x10 0x6AE0 LDR R0,
\ 0x12 0xB100 CBZ.N R0,??GetVoiceID_1
\ 0x14 0x2001 MOVS R0,#+1
\ ??GetVoiceID_1: (+1)
\ 0x16 0xBD10 POP {R4,PC}
...and the new load...
R0,
...to R0 is what the user sees as incorrect. And yes, it would have been incorrect if the variable 'uwElementCnt' would have been 'volatile'. But, as the source was written the compiler, decided to make a 2nd read of 'uwElementCnt' as the compiler was NOT made aware that the value of 'uwElementCnt' could have have been changed in an interrupt. Now looking at what is executes at the end of the function...
\ 0x10 0x6AE0 LDR R0,
\ 0x12 0xB100 CBZ.N R0,??GetVoiceID_1
\ 0x14 0x2001 MOVS R0,#+1
\ ??GetVoiceID_1: (+1)
\ 0x16 0xBD10 POP {R4,PC}
...the load into R0 + CBZ.N (at this location) is in order to check if R0 = 0 and if so the return value (in R0) is set tot 0, and if value loaded in R0 isn't 0, then there is no jump to label...
??GetVoiceID_1
...and the return value in R1 is set to 1.

In other words, given that the variable 'uwElementCnt' isn't 'volatile' the generated sequence of instructions seems to 'do the work' that user had expressed as source.

Now over to how it looks with the variable 'uwElementCnt' marked as 'volatile'.

The list-file looks like:
103 u32 CVoiceBox::GetVoiceID(u32 *puwID)
104 {
\ _ZN9CVoiceBox10GetVoiceIDEPm: (+1)
\ 0x0 0xB510 PUSH {R4,LR}
105 /* NG */
106 if(oVoiceIdFifo.GetUsedLength() > 0)
\ 0x2 0x6AC4 LDR R4,
\ 0x4 0xB114 CBZ.N R4,??GetVoiceID_0
107 {
108 oVoiceIdFifo.Read(puwID);
\ 0x6 0x301C ADDS R0,R0,#+28
\ 0x8 0x.... 0x.... BL _ZN5CFIFOImLi3EE4ReadEPm
109
110 // if cpu reaches here, 1 should be returned
111 // but 0 is returned
112 return 1;
\ ??GetVoiceID_0: (+1)
\ 0xC 0x0020 MOVS R0,R4
\ 0xE 0xBF18 IT NE
\ 0x10 0x2001 MOVNE R0,#+1
\ 0x12 0xBD10 POP {R4,PC}
113 }
114
115 /* OK */
116 // if(oVoiceIdFifo.GetUsedLength() > 0)
117 // {
118 // return oVoiceIdFifo.Read(puwID);
119 // }
120
121 return 0;
122 }

So the call to oVoiceIdFifo.GetUsedLength is still inlined, but now value of 'uwElementCnt' is stored in R4 and there in only 1 read of 'uwElementCnt', as wanted. The value in R4 is compared to 0 in the...
\ 0x4 0xB114 CBZ.N R4,??GetVoiceID_0
...and if zero jumps to label...
\ ??GetVoiceID_0: (+1)
...where the three instructions...
\ 0xC 0x0020 MOVS R0,R4
\ 0xE 0xBF18 IT NE
\ 0x10 0x2001 MOVNE R0,#+1
...sets the return-value in R0 to either 0 ro 1. Thus the compiler follows the 'rules' implied by the 'volatile' keyword.

Summary:
- So originally generated sequence of instrucation was correct as the keyword 'volatile' wasn't used
- And in corrected source the 'uwElementCnt' is only read once, and that at the point of execution where the source places that read operation.
====

MasterPhi 发表于 2022-1-29 00:21:37

SCREA 发表于 2022-1-27 20:11
IAR 多少钱买的?

换算¥第一年2w多,然后每年升级6k多

MasterPhi 发表于 2022-1-29 00:22:06

zchong 发表于 2022-1-28 07:36
目前是什么版本?

9.20.1字数补丁

bigk2000 发表于 2022-1-29 08:47:15

MasterPhi 发表于 2022-1-29 00:19
今天技术支持给我了个超长的回复, 大意如下:

[*] 多个地方会被修改的变量一定要加volatile


是的,加了volatile确实可以解决这个问题,在1出读取后,在2处使用的是1处读取的备份,而不是再读一次
但同时也增加了代码量和降低了执行速度
严格来讲,不加volatile,编译器确实可以再读一次
所以这个bug是有争议的,但我还是更倾向于编译器疏漏

bigk2000 发表于 2022-1-29 08:52:19

MasterPhi 发表于 2022-1-29 00:19
今天技术支持给我了个超长的回复, 大意如下:

[*] 多个地方会被修改的变量一定要加volatile


17楼8.4版本的编译结果是最优化的,省时间,省空间
2处不需要再读,也不需要使用备份值,直接返回就好了
编译器负优化。。。。。

bigk2000 发表于 2022-1-29 08:58:39

All variables that can be changed from interrupts and are used in other parts of the application should be given the keyword 'volatile'.

这条建议合理吗?
中断里的要加volatile没问题,
多个地方修改的变量如果也要加,那程序变得太臃肿了

MasterPhi 发表于 2022-1-29 16:24:59

bigk2000 发表于 2022-1-29 08:58
All variables that can be changed from interrupts and are used in other parts of the application sho ...

他意思应该是说跑rtos的时候多个任务修改的情况

SCREA 发表于 2022-1-29 22:28:27

MasterPhi 发表于 2022-1-29 00:21
换算¥第一年2w多,然后每年升级6k多

这么便宜,给个销售吧
比如邮箱

knight_sh 发表于 2022-1-29 22:45:00

说编译器问题的都很笃定嘛,该用volatile不用,开最高优化出问题了难道不怪你?

MasterPhi 发表于 2022-1-30 18:16:56

SCREA 发表于 2022-1-29 22:28
这么便宜,给个销售吧
比如邮箱

国内销售应该不是一批人,这个邮箱sales.cn@iar.com

tang_qianfeng 发表于 2022-1-31 12:44:19

我觉得编译器在检测到有多个函数要修改的变量,而且这些函数没有顺序关系,就应该不能用寄存器暂存值

MasterPhi 发表于 2022-2-4 18:53:23

你用8.4, 把compiler和linker的list输出都打开,再发一份给我

bigk2000 发表于 2022-2-4 19:40:47

MasterPhi 发表于 2022-2-4 18:53
你用8.4, 把compiler和linker的list输出都打开,再发一份给我

附件为VoiceBox.s,优化最高,size模式,和17楼的图片一致


MasterPhi 发表于 2022-2-18 22:02:49

散了散了, 除了volatile的问题以外, 对这个特定代码9.x的优化确实不行



bigk2000 发表于 2022-2-19 23:17:28

MasterPhi 发表于 2022-2-18 22:02
散了散了, 除了volatile的问题以外, 对这个特定代码9.x的优化确实不行
(引用自49楼)

看来老外借口也不少。。。。{:lol:}

dingrong 发表于 2022-2-22 00:55:22

int fputc(int ch, FILE *f)
{
   HAL_UART_Transmit(&huart2, (uint8_t *)&ch, 1, 0xffff);
    return ch;
}
9.202下,转定义,有问题。大家测试一下看看。

bigk2000 发表于 2022-2-22 19:29:48

dingrong 发表于 2022-2-22 00:55
int fputc(int ch, FILE *f)
{
   HAL_UART_Transmit(&huart2, (uint8_t *)&ch, 1, 0xffff);
(引用自51楼)

这段代码有什么问题,描述一下

dingrong 发表于 2022-2-23 13:13:49

bigk2000 发表于 2022-2-22 19:29
这段代码有什么问题,描述一下
(引用自52楼)

代码没有问题,在最新的iar 9.202版本里执行,串口是哑火的。不动作。试了,其他的方法也是一样,(从网站下载的9.202的历程里,带printf都打印不了。

bigk2000 发表于 2022-2-23 21:21:33

dingrong 发表于 2022-2-23 13:13
代码没有问题,在最新的iar 9.202版本里执行,串口是哑火的。不动作。试了,其他的方法也是一样,(从网 ...
(引用自53楼)

方便把代码传上来,或者做个demo工程?

dingrong 发表于 2022-2-24 23:37:48

bigk2000 发表于 2022-2-23 21:21
方便把代码传上来,或者做个demo工程?
(引用自54楼)

https://www.amobbs.com/forum.php?mod=viewthread&tid=5759352&highlight=fputc   看这个贴子,现象是一样的。iar9.2版本的

anhuicainong 发表于 2022-4-4 15:51:40

MasterPhi 发表于 2022-1-29 00:21
换算¥第一年2w多,然后每年升级6k多
(引用自37楼)

不升级是不是就是2万?

zchong 发表于 2022-4-6 07:36:55

anhuicainong 发表于 2022-4-4 15:51
不升级是不是就是2万?
(引用自56楼)

企业用挺划算了
页: [1]
查看完整版本: 大家来看看,这个是代码不严谨,还是编译器疏漏