[llvm] [GlobalISel] Combine G_ZEXT of undef -> 0 (PR #113764)

[Amara Emerson via llvm-commits]

Thorsten =?utf-8?q?Schütt?= <schuett at gmail.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/113764 at github.com>


================
@@ -2916,8 +2916,11 @@ void CombinerHelper::replaceInstWithFConstant(MachineInstr &MI, double C) {
 
 void CombinerHelper::replaceInstWithConstant(MachineInstr &MI, int64_t C) {
   assert(MI.getNumDefs() == 1 && "Expected only one def?");
-  Builder.buildConstant(MI.getOperand(0), C);
-  MI.eraseFromParent();
+  LLT DstTy = MRI.getType(MI.getOperand(0).getReg());
+  if (isConstantLegalOrBeforeLegalizer(DstTy)) {
----------------
aemerson wrote:

The reason I'm being resistant to your patches in recent times is not because I'm trying to be difficult for the sake of it. My goal is to try to hold the line in quality and overall good engineering practices (with a healthy dose of pragmatism). For AArch64 this framework is supposed to be the replacement for SelectionDAG in the long term future and we may have the design being used to compile trillions of lines of code, for software that's used directly or indirectly by almost every person on earth. The weight of that responsibility means that I would rather we not fix issues/add features than to degrade the quality of our codebase with a fatal thousand cuts. A quick fix now that compromises design cleanliness could bite us 10x worse in the future.

And despite the above, I'm not opposed to fixing issues on an iterative basis. We don't have to reach the final destination in one go, but it would be good to have some sort of plan. E.g. "I know this isn't an ideal solution, but I think the trade offs are worth it, and I'm willing to rethink the design in future" is the mindset I want to cultivate.

https://github.com/llvm/llvm-project/pull/113764