[PATCH] D104080: [LLD][LLVM] CG Graph profile using relocations

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: lld/ELF/Driver.cpp:867
+    if (obj->cgProfileRela.size() != obj->cgProfile.size() * 2)
+      fatal("Number of relocations doesn't match Weights.");
+    for (uint32_t i = 0, size = obj->cgProfile.size(); i < size; ++i) {
----------------
drop trailing period

Don't capitalize


================
Comment at: lld/ELF/Driver.cpp:866
     auto *obj = cast<ObjFile<ELFT>>(file);
-
-    for (const Elf_CGProfile_Impl<ELFT> &cgpe : obj->cgProfile) {
-      auto *fromSym = dyn_cast<Defined>(&obj->getSymbol(cgpe.cgp_from));
-      auto *toSym = dyn_cast<Defined>(&obj->getSymbol(cgpe.cgp_to));
+    assert(obj->cgProfileRela.size() == obj->cgProfile.size() * 2 &&
+           "Number of relocations doesn't match Weights.");
----------------
ayermolo wrote:
> MaskRay wrote:
> > This can happen so assert is not appropriate. You can use `if (...) fatal`
> What is general guideline for fatal vs assert in llvm? I looked at Coding Standard, and didn't see anything (might have missed it)
My personal guideline is:

assert is for ensuring an invariant doesn't break - it shouldn't break even for malicious user input.

If you know some user input would crash, prefer a proper error reporting mechanism, in lld errorOrWarn/fatal/error. In LLVM internal code you can find many `report_fatal_error`(similar to assert, just with diagnostic even when assertions is disabled). Many are lazy examples which are not good references.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104080/new/

https://reviews.llvm.org/D104080