[llvm] [compiler-rt] [clang] [Profile] Refactor profile correlation. (PR #70856)

2023-11-10 Thread Zequan Wu via cfe-commits

ZequanWu wrote:

Chatted with @MaskRay offline, we come to an agreement that it's not good to 
use relying on the bit in __llvm_profile_raw_version to decide whether or not 
to dump data/name sections at runtime for the reasons mentioned above. Sent a 
PR: #71996

> Even if we have a way to fix that, we still need the mode bit at llvm-profile 
> merging step to indicate if the raw profile need debug info/binary to 
> correlate. However, we can get ride of the runtime checking 
> __llvm_profile_has_correlation to allow code compiled w/wo debug-info linked 
> together works well.

I take back this word. When we do `llvm-profdata merge`, we use `-debug-info` 
to provide debug info file for correlation and the same could apply to binary 
correlation, a different flag. So, we know which correlation mode we are using.

https://github.com/llvm/llvm-project/pull/70856
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[llvm] [compiler-rt] [clang] [Profile] Refactor profile correlation. (PR #70856)

2023-11-10 Thread Fangrui Song via cfe-commits

MaskRay wrote:

I am nervous with new `if (hasCorrelation())` conditions as well.


If you use the same name for a non-SHF_ALLOC section and a SHF_ALLOC section, 
the linked output has the SHF_ALLOC flag and you cannot tell what components 
were non-SHF_ALLOC.
```
.section aa,"",@progbits,unique,1
.quad 0

.section aa,"a",@progbits,unique,2
.quad 0
```

Usually, the better idea is to use different section names. It may require some 
trial and error to see how much complexity this scheme will bring.

> I feel like this is a bug in lld as _start/_stop symbols for non allocated 
> sections should be null. 

It isn't. This is "undefined behavior, no diagnostic required".

For
```
extern char __start___llvm_covfun __attribute__((weak));
extern char __stop___llvm_covfun __attribute__((weak));

int main() {
  printf("%p, %p\n", &__start___llvm_covfun, &__stop___llvm_covfun);
  return 0;
}
```

The program is ill-formed when `__llvm_covfun` does not have the SHF_ALLOC flag.
`__start_`/`__stop_` symbols were not designed to be used with non-SHF_ALLOC 
output sections.
It's invalid to reference a non-SHF_ALLOC section from SHF_ALLOC code. The 
regular undefined weak rule (handwavy "Unresolved weak symbols have a zero 
value." in the specification) does not necessarily apply.

I noticed that GNU ld defines `__start_`/`__stop_` as well and its linked a.out 
prints non-zero addresses, just like lld.


https://github.com/llvm/llvm-project/pull/70856
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits