[PATCH] D45157: [RegUsageInfoCollector] Don't assume the alias of a defined reg is always already in the set.

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa added a comment.

In https://reviews.llvm.org/D45157#1075960, @qcolombet wrote:

> I am not sure I like the patch in the sense that the comment for UsedPhysRegsMask (in ’MachineRegisterInfo.h) explicitly said that it should contain all the aliases.




  /// UsedPhysRegMask - Additional used physregs including aliases.                                                                                                                                                              
  /// This bit vector represents all the registers clobbered by function calls.                                                                                                                                                  
  BitVector UsedPhysRegMask;

I read this not as all aliases of all registers are included, but merely those actually clobbered. In the test case, fn1 clobbers %r0l, so %r0l and it's aliases (%r0d and %r0q) are also in UsedPhysRegMask. However, %r0h is *not* clobbered (the other subreg of %r0d).

> So either, we change this assumption (which that patch does) and we update the patch to change the comment *and* we make sure it does not break anything else if other targets start to not put the aliases in that set, or we fix that target to add the aliases when it creates the set.
> 
> From a high level point of view, it looks riskier to take the first approach. Can't we change the code that set this mask in SystemZ?
> 
> @jonpa from the PR, I didn't get if the mask was "wrong" before RegCollectUsage starts or if it is RegCollectUsage that fails to update it. IIRC, RegCollectUsage does not change UsedPhysRegsMask, so I was assuming the state was wrong to begin with.

My understanding is that when RegInfoUsageCollector looks at @fn1, it only adds %r0l (and superregs), but not %r0h. @fn1 only clobbers %r0l.  @fn2 however does clobber %r0d, which is already in the set after visiting @fn1. So here it is not true that all aliases of %r0d are in the set!

I am not aware that the backend should set this mask somehow, I have so far assumed that it is the RegCollectUsage/Propagate passes that sets the call regmask.

Perhaps we should change the comment to "... including clobbered aliases..." ?


https://reviews.llvm.org/D45157