Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

i#1569 AArch64: Make encodings easier to specify. #2549

Merged
merged 3 commits into from
Jul 20, 2017
Merged

Conversation

egrimley
Copy link
Contributor

@egrimley egrimley commented Jul 19, 2017

In the operand definitions, in codec.txt, '?' indicates a bit that
the operand decoder/encoder may consult as it decodes or encodes the
bits labelled 'x'. An operand encoder only receives the bits
labelled '?'; an operand decoder only receives the bits labelled '?'
or 'x'. An operand encoder may generate a bit that is read by another
operand encoder, provided there is no cyclic dependency. This change
makes it easier to specify some encodings, and to detect errors.

A few changes are made exploiting the new facilities:

  • memvrpost is removed: memvr is sufficient
  • some patterns for SIMD load/store structure are combined
  • some patterns for data processing with shifted register are combined
    (for which "imm6" is modified: it now checks bit 31 for consistency)

Change-Id: Ic5ef256af143239521c4cea86631dabebd763ea1

In the operand definitions, in codec.txt, '?' indicates a bit that
the operand decoder/encoder may consult as it decodes or encodes the
bits labelled 'x'. An operand encoder only receives the bits
labelled '?'; an operand decoder only receives the bits labelled '?'
or 'x'. An operand encoder may generate a bit that is read by another
operand encoder, provided there is no cyclic dependency. This change
makes it easier to specify some encodings, and to detect errors.

A few changes are made exploiting the new facilities:

- memvrpost is removed: memvr is sufficient
- some patterns for SIMD load/store structure are combined
- some patterns for data processing with shifted register are combined

Change-Id: Ic5ef256af143239521c4cea86631dabebd763ea1
@egrimley egrimley requested a review from derekbruening July 19, 2017 10:46
@egrimley
Copy link
Contributor Author

AppVeyor failure looks like #2246.

@@ -1128,12 +1128,16 @@ encode_opnd_imm5(uint enc, int opcode, byte *pc, opnd_t opnd, OUT uint *enc_out)
static inline bool
decode_opnd_imm6(uint enc, int opcode, byte *pc, OUT opnd_t *opnd)
{
if (!TEST(1U << 31, enc) && TEST(1U << 15, enc))
return false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I see where this is coming from. This is a bug fix? Should be mentioned in the commit message? Same below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has to do with "data processing with shifted register". I've added a parenthesis to the draft log message ("comment") above.

for i in range(len(srcs))]
c += [' return true;',
'}',
'']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is all just cosmetic?

gen(c, pats1, depth + 1)
c.append('%s}' % indent)
c += ['%s}' % indent]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of these s/append/+=/ seem to confuse the issue here: unrelated to the content changes, right? Maybe better to be a separate CL.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll undo the cosmetic changes for now.

Unrelated addition to log message:
  (for which "imm6" is modified: it now checks bit 31 for consistency)

Change-Id: I194956ff90ce2f039f350ec057271e00e97da192
@egrimley egrimley merged commit c908cdd into master Jul 20, 2017
@egrimley egrimley deleted the i1569-codec branch July 20, 2017 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants