[llvm] [nfc][InstrProf]Remove 'offsetOf' when parsing indexed profiles (PR #93346)
Mingming Liu via llvm-commits
llvm-commits at lists.llvm.org
Tue May 28 15:38:14 PDT 2024
minglotus-6 wrote:
> Thank you for cleaning up these things! I like where things are going overall. Now, would you mind creating focused patches that do one thing.
Sure. I created https://github.com/llvm/llvm-project/pull/93613 and https://github.com/llvm/llvm-project/pull/93594.
>
> * I think the `GET_VERSION` clean up should call a method in `Header`. If `Header->getVersion()` is confusing, you might say `Header->getIndexedProfileVersion()` or something. I wouldn't save the version in `ProfileVersion`. Instead, I would keep calling `Header->getIndexedProfileVersion()`.
https://github.com/llvm/llvm-project/pull/93613 does this.
> * `read_and_decrement` should be named `readAndDecrment` or `readAndDecrement` according to the LLVM Coding Standards. Now, I feel a bit uneasy about going backward. Let's discuss this in a separate patch.
I did hesitate between reading forward and backward when write the partial forward compatibility patch (https://github.com/llvm/llvm-project/pull/88212), and chose to retain backward parsing mainly because existing code prefers `switch` with fallthroughs.
https://gcc.godbolt.org/z/qdM4naeM1 roughly demonstrated the IR for both `switch-with-fallthrough` and a couple of `if` statements. Since this is not performance critical code and the thoughts I hear so far (here, and in a chat room which discusses code patterns/styles) prefer to read forward, I updated the code to use `readNext`.
https://github.com/llvm/llvm-project/pull/93346
More information about the llvm-commits
mailing list