[llvm] r361912 - [X86] Use 'llvm_unreachable' instead of nullptr in unreachable code to

[Wang, Pengfei via llvm-commits]

Hi David,

Thanks for your advice! However, I have reverted this change. We found some unit tests failed due to this change, and Quentin also explained the reason why it becoming reachable sometime. https://reviews.llvm.org/D62006
Except that, I still prefer to the former code. It is friendly to people unfamiliar with this code for it implicitly showing RegisterBank has the same weight as TargetRegisterClass.

Thanks
Pengfei

From: David Blaikie [mailto:dblaikie at gmail.com]
Sent: Tuesday, June 4, 2019 3:39 AM
To: Wang, Pengfei <pengfei.wang at intel.com>
Cc: llvm-commits <llvm-commits at lists.llvm.org>
Subject: Re: [llvm] r361912 - [X86] Use 'llvm_unreachable' instead of nullptr in unreachable code to



On Tue, May 28, 2019 at 7:17 PM Pengfei Wang via llvm-commits <llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>> wrote:
Author: pengfei
Date: Tue May 28 19:20:37 2019
New Revision: 361912

URL: http://llvm.org/viewvc/llvm-project?rev=361912&view=rev
Log:
[X86] Use 'llvm_unreachable' instead of nullptr in unreachable code to
avoid static check fail

RegClassOrBank is an object of RegClassOrRegBank, which is defined as
using llvm::RegClassOrRegBank = typedef PointerUnion<const
TargetRegisterClass *, const RegisterBank *>
so control flow can not get here. Use ""llvm_unreachable" here to avoid
"null pointer" confusion.

Patch by Shengchen Kan (skan)

Differential Revision: https://reviews.llvm.org/D62006

Signed-off-by: pengfei <pengfei.wang at intel.com<mailto:pengfei.wang at intel.com>>

Modified:
    llvm/trunk/lib/CodeGen/GlobalISel/RegisterBankInfo.cpp
    llvm/trunk/lib/Target/X86/X86InstructionSelector.cpp

Modified: llvm/trunk/lib/CodeGen/GlobalISel/RegisterBankInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GlobalISel/RegisterBankInfo.cpp?rev=361912&r1=361911&r2=361912&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/GlobalISel/RegisterBankInfo.cpp (original)
+++ llvm/trunk/lib/CodeGen/GlobalISel/RegisterBankInfo.cpp Tue May 28 19:20:37 2019
@@ -91,7 +91,9 @@ RegisterBankInfo::getRegBank(unsigned Re
     return RB;
   if (auto *RC = RegClassOrBank.dyn_cast<const TargetRegisterClass *>())
     return &getRegBankFromRegClass(*RC);
-  return nullptr;
+
+  llvm_unreachable("RegClassOrBank is either a const RegisterBank* or "
+                   "a const TargetRegisterClass*");

Could simplify this ^ further to:

return &getRegBankFromRegClass(*RegClassOrBank.get<const TargetRegisterClass*>());

(since the 'get' will assert the same way the unreachable would've - though if you prefer the more specific message you can provide with the unreachable, that's OK too)
 }

 const TargetRegisterClass &

Modified: llvm/trunk/lib/Target/X86/X86InstructionSelector.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstructionSelector.cpp?rev=361912&r1=361911&r2=361912&view=diff
==============================================================================
--- llvm/trunk/lib/Target/X86/X86InstructionSelector.cpp (original)
+++ llvm/trunk/lib/Target/X86/X86InstructionSelector.cpp Tue May 28 19:20:37 2019
@@ -1610,8 +1610,8 @@ bool X86InstructionSelector::selectDivRe
   assert(RegTy == MRI.getType(Op1Reg) && RegTy == MRI.getType(Op2Reg) &&
          "Arguments and return value types must match");

-  const RegisterBank *RegRB = RBI.getRegBank(DstReg, MRI, TRI);
-  if (!RegRB || RegRB->getID() != X86::GPRRegBankID)
+  const RegisterBank &RegRB = *RBI.getRegBank(DstReg, MRI, TRI);
+  if (RegRB.getID() != X86::GPRRegBankID)
     return false;

   const static unsigned NumTypes = 4; // i8, i16, i32, i64
@@ -1709,7 +1709,7 @@ bool X86InstructionSelector::selectDivRe
   const DivRemEntry &TypeEntry = *OpEntryIt;
   const DivRemEntry::DivRemResult &OpEntry = TypeEntry.ResultTable[OpIndex];

-  const TargetRegisterClass *RegRC = getRegClass(RegTy, *RegRB);
+  const TargetRegisterClass *RegRC = getRegClass(RegTy, RegRB);
   if (!RBI.constrainGenericRegister(Op1Reg, *RegRC, MRI) ||
       !RBI.constrainGenericRegister(Op2Reg, *RegRC, MRI) ||
       !RBI.constrainGenericRegister(DstReg, *RegRC, MRI)) {


_______________________________________________
llvm-commits mailing list
llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190604/4f09b18a/attachment.html>