ZarkoCA marked 6 inline comments as done. ZarkoCA 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) ---------------- sfertile wrote: > 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. No, you were right those changes can't be tested. Just needed some clarification, thanks. 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
