sfertile added inline comments.

================
Comment at: llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp:235
+    return TM.isPPC64()
+               ? (Subtarget.hasAltivec() ? CSR_64_AllRegs_Altivec_RegMask
+                                         : CSR_PPC64_RegMask)
----------------
ZarkoCA wrote:
> sfertile wrote:
> > `CSR_64_AllRegs_Altivec_RegMask` should be `CSR_PPC64_Altivec_RegMask`.  
> > FWIW I don't think this is testable without D86476. If that's the case, 
> > then it should go in that patch, not this patch. 
> Are you suggesting that I also leave the error in if I were to move this 
> change to D84676? 
Can you still run the tests that are part of this commit with that error in? My 
understanding was that it didn't interfere, but I didn't verify that. If we can 
still run the tests then yes leave the error in. If we can't then it probably 
gives us a clue about how to test the change in this patch without needing 
D84676, in which case we can keep the change and simply add the testing that 
exercises it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88676

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to