OpenXiangShan / NEMU

Other
236 stars 90 forks source link

build: disable march=native when building as ref so #464

Closed shinezyy closed 2 months ago

shinezyy commented 2 months ago
poemonsense commented 2 months ago

I just noticed the flto option in the flags.

This is previously controller by a config option (https://github.com/OpenXiangShan/NEMU/blob/master/Makefile#L80). Should we add it in the build.mk?

Maybe the same can apply to march=native. For example, add a config for the march?

shinezyy commented 2 months ago

I just noticed the flto option in the flags.

This is previously controller by a config option (https://github.com/OpenXiangShan/NEMU/blob/master/Makefile#L80). Should we add it in the build.mk?

Maybe the same can apply to march=native. For example, add a config for the march?

I agree that march should be controlled.

But Why flto should be controlled? Is it harmful on some situations?

shinezyy commented 2 months ago

I found a discussion on flto here: https://stackoverflow.com/questions/23736507/is-there-a-reason-why-not-to-use-link-time-optimization-lto

An answerer listed several disadvantages of lto in 2014. But he said: "As of 2020, I would try to use LTO by default on any of my projects."

So I suggest to turn on lto by default while control march with KConfig.

poemonsense commented 2 months ago

The config is introduced in 2021 (https://github.com/OpenXiangShan/NEMU/commit/1cd4b2b55bba94851fb23a835bade645840c45a0). Maybe yzh requires this option for his research evaluation of NEMU at that time.

I had one issue on LTO last year when using Clang coverage instrumentation (https://github.com/OpenXiangShan/NEMU/commit/f04c4ff72ab90f1cd0a065c78716805e6d8a27c1). If I remembered it correctly, building the so with flto will remove unused functions, and then the instrumented coverage cannot be accessed.

You may try enabling flto by default. I'll test it in the future when I have free time to see whether it now works for fuzzing.

poemonsense commented 2 months ago

BTW, ref-so does not really care about performance. The RTL runs much slower than NEMU-so. So maybe disabling lto does not bring visible performance degradation.

But I'm ok with disabling/enabling it by default

shinezyy commented 2 months ago

BTW, ref-so does not really care about performance. The RTL runs much slower than NEMU-so. So maybe disabling lto does not bring visible performance degradation.

But I'm ok with disabling/enabling it by default

Yes, I think I only need to enable it for default config of xs and xs cpt confg.