Improve handling of AllowLDSDMA in SIInstrInfo.isVALU
- Dominant language
- LLVM
- Stars
- 40.5k
- Forks
- 18.7k
- PR merge metrics
- PR metrics pending
Description
https://github.com/llvm/llvm-project/pull/203548 added a flag `AllowLDSDMA` to give users of isVALU the ability to not pick up LDSDMA instructions as well. While this is a convenience, the main intention was to make users aware that isVALU picked these instructions up.
In https://github.com/llvm/llvm-project/pull/203548#issuecomment-4710142559 we discussed the direction we should take this. In short, the next steps are to:
- always set `AllowLDSDMA` to false -- this implies taking a look at the users of this method and possibly changing the logic around the users to reflect isVALU no longer picks these up. For example, we may have some clients which rely on this behavior. Commits that set `AllowLDSDMA` to false should either be: 1. NFC or 2. bug fixes (where we erroneously picked up LDSDMA without meaning to).
- explore the possibility of no longer marking LDSDMA as VALU and removing this flag entirely from isVALU
Contributor guide
Research direction
Start by tracing the users of SIInstrInfo.isVALU and every AllowLDSDMA argument, then read the discussion in pull request 203548 for the intended behavior. Done means the callers consistently reflect the chosen LDSDMA handling and the remaining behavior is covered by the relevant LLVM tests, if identified while tracing those users.
Written by the indexing model from the issue text.
Assessment
- Domain
- compilers
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100