i#1569 AArch64: Fix bug in encoding (SIMD structure load/store). #2527
Conversation
The "x16imm" operand type can be a register, or a constant denoted in the encoding by register number 31. The value of the constant can be determined from various bits in the instruction, but those bits can not be uniquely determined from the constant operand. In particular, we can not generate bit 30 in this case. Within the constraints of the current formalism the best solution seems to be to remove bit 30 from the bits generated by x16imm and duplicate the 14 bit patterns that use x16imm. There are other instances of such duplication. The formalism should perhaps be improved. This patch also adds some unnecessary initialisers to prevent certain compiler versions from breaking the build by incorrectly claiming that certain variables "may be used uninitialized". Change-Id: I319d8e4b9550d39b11f5dce80fe7801115445446
|
run aarch64 tests |
core/arch/aarch64/codec.py
Outdated
| @@ -135,7 +135,8 @@ def generate_encoder(patterns, opndsgen, opndtypes): | |||
| vars = (['dst%d' % i for i in range(len(dsts))] + | |||
| ['src%d' % i for i in range(len(srcs))]) | |||
| c += [' int opcode = instr->opcode;'] | |||
| c += [' uint ' + ', '.join(vars) + ';'] | |||
| # The initial values are only required to silence a bad compiler warning: | |||
| c += [' uint ' + ' = 0, '.join(vars) + ' = 0;'] | |||
There was a problem hiding this comment.
I think it would be cleaner to add the initialisers in a separate commit.
There was a problem hiding this comment.
You're probably right.
There was a problem hiding this comment.
Separate pull request for initialisers: #2528
|
run aarch64 tests |
|
Would it be hard to add a test case for that, e.g. encoding the newly added variants? I think this would be a helpful regression test if we decide to make changes to the formalism you hinted at. Also the following sentence should be removed from the final commit message. |
|
Yes, it ought to be possible to add a line to |
|
I'll delete the last paragraph from the description. Presumably I'll have to delete it again when I squash and merge. |
Change-Id: I2b83318135bd5e6c736aff1ff330e838417fa3d2
|
rerun aarch64 tests |
The "x16imm" operand type can be a register, or a constant denoted
in the encoding by register number 31. The value of the constant can
be determined from various bits in the instruction, but those bits
can not be uniquely determined from the constant operand. In particular,
we can not generate bit 30 in this case. Within the constraints of the
current formalism the best solution seems to be to remove bit 30 from
the bits generated by x16imm and duplicate the 14 bit patterns that
use x16imm.
There are other instances of such duplication. The formalism should
perhaps be improved.
Change-Id: I319d8e4b9550d39b11f5dce80fe7801115445446