[llvm] [GlobalIsel] Combine G_ADD and G_SUB with constants (PR #97771)

Amara Emerson via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 6 21:54:58 PDT 2024


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


aemerson wrote:

> Firstly, please update the coding style section of the Combiner.rst with the outcome of the discussion. I don't like the "If the instruction is _**known**_ to be a G_CONSTANT ". The I know what I am doing might fail badly. Warning does not compile:
> 
> ```
> APInt getIConstantFromReg(Register Reg, const MachineRegisterInfoI& MRI) {
>   return cast<GIConstant>(MRI.getVRegDef(Reg))->getAPInt();
> }
> ```
> 
> Bugs will make it fail badly. `getIConstantVRegVal` is more verbose, but gives better protection against errors.
> 
> In terms of compile-time, the next step is to hoist the oneuse checks into the MIR patterns.

If it's expected that there is always a value in an optional or the compiler is in a bad internal state, then just bailing out and skipping the combine isn't solving anything. In fact I'd argue is actively harmful since it hides what should be a fatal error/assertion failure.

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


More information about the llvm-commits mailing list