For me the patch looks good, thank you for working on it Alex!

@asb I just missed my opportunity in the call to ping you regarding this patch. ;) Are there any remaining issues with it that I missed?

Updated the patch to emit ADDIW again and rebased it.

Thank you for finding the flaw in my thinking. I obviously missed that everything falls apart due to the sign extension of the LUI instruction...

In case it helps, my reasoning is as follows. There are basically two situations which can occur:

Rebased the patch. Luckily, it was not as bad as I anticipated. :)

Ping! Some feedback would be nice before I rebase this patch (which is less fun as it should be due to the recent improvements on the ASM parser)...

I finally found the time to work on this patch, sorry for the delay.

Hi Sameer,

In D41949#1069751, @asb wrote:

I'll think more about compression handling. If you already have something that works, it might be worth just posting that so we have something concrete to discuss.

Updated patch to fix variable names.

In D41949#1067297, @asb wrote:

I'd do the compressed changes in a different patch. Thanks for updating the peephole RISCVISelDAGToDAG, I'll review that bit ASAP and then commit. At a first look, it seems to handle this exactly as I would expect.

Extended peephole optimisation to fix introduced codegen regression.

Rebased on master as Mandeep requested via email .

In D41949#1045516, @asb wrote:

Thanks Mario. I think this is looking good to land now.

I rebased the patch and addressed all comments. Thank you again for the feedback.

Hi Alex, thank you very much for your comments!

Thank you for your comments Eli!

Addressed the discovered defect regarding the immediate of the li instruction. In RV32 mode we now accept either a signed or an unsigned 32-bit value. In RV64 mode we accept basically everything that fits into 64-bit.

I just stumbled across a difference between the binutils assembler and my current li implementation regarding accepted immediate values.

Fixed some typos in the comments.

Added support for handling 64-bit immediate values.

Thank you Ana for your comments!

Moved the shared emitLoadImm methods to a free function into the RISCVDesc library. Additionally, the Size of the pseudo instruction has been set to 8 (worst case for RV32) to ensure proper branch relaxation. Finally, the pattern for 32-bit immediate integers has been updated to generate PseudoLI and the tests have been updated accordingly.

Addresses all of Alex's comments (thank you) and integrates PseudoLI emission into CodeGen.

Hi Alex, thank you for your comments!

Great, thank you Ana for confirming that this patch fixes the problem.

Ignoring the predicates currently leads to the problem that aliases get printed although the corresponding extension is not enabled.

Great. I am going to work on the patch which enables aliases by default and updates the tests next. Is it preferred that only the RUN lines are updated to disable aliases in the already existing tests or should the actual content of the tests be updated with aliases?

Addressed all comments for this patch. I already used riscv-no-aliases as command line flag but defaulted it to true instead of adding riscv-aliases which would be removed soon anyway.

Thank you for the review Alex. I will incorporate your comments in an updated patch today.

Rebased as requested.