https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97417

--- Comment #13 from Jim Wilson <wilson at gcc dot gnu.org> ---
The attachments show the entire riscv.c file being deleted and then readded
with your change.  This is due to a line ending problem.  The original file has
the unix style linefeed and the new file has the windows style carriage return
and linefeed.  Git has a setting called core.autocrlf that can help with this. 
This will require using git diff instead of just diff though to generate
patche, but should give better diffs.  Or alternatively, maybe to can force the
original file to have msdos line endings before you make the diff, e.g. maybe
just loading the original file into your editor and saving it without making
changes will fix the line endings.

You have in the second patch
    (mode == QImode || mode == SImode || mode == HImode)
which is wrong but harmless for rv32 since we can't extend SImode, and is also
wrong for the eventual rv128 support.  You can fix this by using something like
    (GET_MODE_CLASS (mode) == MODE_INT && GET_MODE_SIZE (mode) < UNITS_PER_WORD
There is one place in riscv_legitimize_move that already uses this.

The code inside the if statement is a lot more verbose then necessary.  You can
use some helper functions to simplify it.  First you can use word_mode which is
DImode for rv64 and SImode for rv32 (and will be OImode for rv128).  Then you
can call a helper to do the mode conversion.  So for instance something like
  temp_reg = force_reg (word_mode, convert_to_mode (word_mode, src, 1));
should work.  That should emit an insn to do the zero extend and put it in a
reg.  Now you no longer need to check src mode or TARGET_64BIT as the code is
the same in all cases.  So it should just be about 3 or 4 lines of code for the
body of the if statement.

You have a check for REG_P (dest).  This is stricter than you need, since it
only works for REG and doesn't accept SUBREG.  We should handle both.  Also, it
isn't hard to also handle non-reg or non-subreg dests.  You just need to force
the source to a reg, and you already did that when you generated the
zero_extend operation.  So this code looks like it should work for any dest and
you should be able to drop the REG_P (dest) check.

Reply via email to