[llvm-commits] [PATCH, PowerPC] Fix MC instruction encodings
Hal Finkel
hfinkel at anl.gov
Tue Nov 13 11:01:08 PST 2012
----- Original Message -----
> From: "Ulrich Weigand" <Ulrich.Weigand at de.ibm.com>
> To: llvm-commits at cs.uiuc.edu
> Sent: Monday, November 12, 2012 1:08:23 PM
> Subject: [llvm-commits] [PATCH, PowerPC] Fix MC instruction encodings
>
>
> Hello,
>
> attempting to run the test-suite using the integrated assembler
> instead of
> the external assembler shows a couple of wrong instruction encodings.
> The
> attached four patches fix all of those; if they're all applied (and
> the TOC
> order patch), there are no significant differences (at least in the
> .text
> section) between objects built with the external and the integrated
> assembler any longer, except for one remaining problem: the
> integrated
> assembler currently does not generate thread-local storage
> relocations.
>
> The four patches fix the following problems:
>
> 1. isel encoding
> The encoding for the isel instruction seems to have been completely
> wrong
> ever since the pattern was added. The patch adds a new instruction
> format
> AForm_4 to handle this instruction.
>
> 2. bd(n)z encoding
> This instruction likewise seems to have been wrong all along; it uses
> a
> format IForm_ext which does:
>
> class IForm_ext<bits<6> opcode, bits<5> bo, bit aa, bit lk, dag OOL,
> dag
> IOL,
> string asmstr, InstrItinClass itin, list<dag> pattern>
> : IForm<opcode, aa, lk, OOL, IOL, asmstr, itin, pattern> {
> let LI{0-4} = bo;
> }
>
> This doesn't work since {0-4} affects the *low* bits of LI, and the
> value
> needs to be placed at the high bits. In any case, it seems weird
> to use
> a variant of IForm for bd(n)z as those are prototypical BForm
> patterns; for
> example, the above pattern also does not attempt to set BI at all.
> My
> patch changes this to use a variant of BForm for this instruction.
>
> 3. Operand name mismatches
> In the past, is seems that instruction operands were matched onto
> field
> names in the instruction format strictly by order. A while ago, a
> patch
> came in that changed this to try to look for a matching name first,
> and
> only use order to match those fields whose name doesn't match any
> operand
> name. It appears the PowerPC back-end was never updated to
> accommodate
> that change.
>
> Now, mostly this doesn't matter because none of the field names match
> any
> operand name anyway. In some cases where there is a match, they so
> happen
> to occur in the correct order, so it doesn't hurt either. However,
> there
> are a couple of places where now operands are matched in the wrong
> sequence. The patch fixes this for the following formats:
> - AForm_3 (fmul, fmuls)
> - XFXForm_5 (mtcrf)
> - XFLForm (mtfsf)
>
> [I guess to avoid confusion, it might be good in the long term to
> change
> all operand names to match field names, so we never rely on order any
> more ...]
>
> 4. Wrong opcodes
> A couple of encoding just plain use the wrong (major or minor)
> opcode, and
> thus generate the wrong instruction completely. The patch fixes
> this for:
> - lwaux
> - lhzux
> - stbu
>
>
> OK to commit?
LGTM. We should add test cases (but I think that we can't do that until we have the asm parser working, and that's blocked by TableGen problems), so those can wait.
Thanks again,
Hal
>
> Bye,
> Ulrich
> (See attached file: diff-llvm-mc-encode-isel)
> (See attached file: diff-llvm-mc-encode-bdz)
> (See attached file: diff-llvm-mc-encode-opnames)
> (See attached file: diff-llvm-mc-encode-opcodes)
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
--
Hal Finkel
Postdoctoral Appointee
Leadership Computing Facility
Argonne National Laboratory
More information about the llvm-commits
mailing list