[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