[PATCH] D54882: [AMDGPU] Add sdwa support for ADD|SUB U64 decomposed Pseudos

[Ron Lieberman via Phabricator via llvm-commits]

ronlieb marked an inline comment as done.
ronlieb added inline comments.


================
Comment at: lib/Target/AMDGPU/SIPeepholeSDWA.cpp:914
+  // Check if VCC is referenced in range of (MI,MISucc].
+  for (MachineBasicBlock::const_iterator I = &*std::next(MI.getIterator());
+       I != MISucc; ++I) {
----------------
ronlieb wrote:
> rampitec wrote:
> > Ugh.. Why do you need an interator from iterator?! This may even fail if next is end(). std::next() already returns you an iterator.
> The short answer is i have not found another mechanism to increment I to the next MI, that both compiles and does not abort.
> 
> In this particular context, MI and MISucc are both references to valid instructions in the basic block, and further that MISucc depends on MI,so its after MI in the instruction iteration order. 
> 
> This loops is not executed unless we have found both MI and MISucc, so that means we will not see MBB.end() as we will give up upon encountering MISucc.
> 
> Further , in practice, MI and MISucc are very close to each other, if not actually next o each other.
> 
> All that said, the savings from calling modifiesRegister one extra time is hopefully not too expensive in practice.
> 
> My preference at this point is to use one of the two following:
> personally, i think #1 is the simplest.
> 
> 1) 
> +  // Check if VCC is referenced in range of MI and MISucc.
> +  for (MachineBasicBlock::const_iterator I = MI; I != MISucc;
> +       I = std::next(I)) {
> +    if (I->modifiesRegister(AMDGPU::VCC, TRI))
> 
> 2)
> +  // Check if VCC is referenced in range of (MI,MISucc].
> +  MachineBasicBlock::const_iterator I = MI;
> +  for (++I; I != MISucc; ++I) {
> +    if (I->modifiesRegister(AMDGPU::VCC, TRI))
> 
> 
i need to use iterator instead of const_iterator

  // Check if VCC is referenced in range of (MI,MISucc].
  for (MachineBasicBlock::iterator I = std::next(MI.getIterator());
       I != MISucc; ++I) {



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

https://reviews.llvm.org/D54882