[PATCH] D145282: [LLParser] Error out if a name is too long and gets renamed

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added a comment.

In D145282#4168678 <https://reviews.llvm.org/D145282#4168678>, @arsenm wrote:

> I’d expect the names to be truncated to the limit plus the conflict resolution number. Does the limit clamp the suffix?

yes that is how it works, the limit doesn't clamp the suffix, but that's not the issue

the issue is that within LLParser, `F.getValueSymbolTable()->lookup()` truncates the name, resulting in both `testa` and `testz` getting mapped to the same BasicBlock in `LLParser::PerFunctionState::getVal`

the original issue was https://github.com/llvm/llvm-project/issues/45244, which https://reviews.llvm.org/D102707 sort of fixed. but the issue of collisions was never raised and I ran into it with very long basic block names. I think it's possible to fully fix this issue by keeping track of renaming but it's not trivial (e.g. have to handle the case where we see `testz`, truncate it to `test` internally, then see an actual `test` later on), and a solution wouldn't be free in terms of compile time


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145282