[llvm-commits] [PATCH] thumb t2b fix

[Greg Fitzgerald]

Owen,

I think we can use more guidance on how to write the .td file for this
instruction.  The encodings for bit 13, 11, and 26 are J1, J2 and S
respectively.  ARM explains how to decode the instruction as:

I1 = NOT(J1 EOR S);
I2 = NOT(J2 EOR S);
imm32 = SignExtend(S:I1:I2:imm10:imm11:'0', 32);

Is there a best practice for describing this moderately-complex bit of
encoding-decoding symmetry in the .td file?  Do we need to call two
separate external functions, or is there some boomerang-like solution
to stay DRY?

Thanks,
Greg


On Tue, Sep 18, 2012 at 5:36 PM, Owen Anderson <resistor at mac.com> wrote:
> Greg,
>
> This ties in with my earlier comments: the immediate MCOperand on the MCInst is always assumed to contain a pre-encoded value.  The purpose of this is to ensure that it is not possible to represent an unencodable instruction as an MCInst.
>
> --Owen
>
> On Sep 18, 2012, at 5:20 PM, Greg Fitzgerald <garious at gmail.com> wrote:
>
>> A small nitpick from an llvm newb, why these two lines?
>>
>> +  let Inst{13} = target{22};
>> +  let Inst{11} = target{21};
>>
>> These 2 bits require separate logic, which you have.  So should we be
>> sure not to reference them in the .td file?  Or better, is there a way
>> to do the EOR of S and J1 within the .td file?
>>
>> Thanks,
>> Greg
>>
>>
>> On Tue, Sep 18, 2012 at 9:04 AM, Tim Northover <t.p.northover at gmail.com> wrote:
>>>
>>> Hi all,
>>>
>>>> Please review and commit this patch.
>>>>
>>>> It fixes the thumb t2b instruction, where the immediate value was being
>>>> incorrectly encoded and decoded.
>>>
>>> I can do the committing again, but is there any chance we could have
>>> some non-ARM review on this?
>>>
>>> I think it fixes the important codegen errors around the Thumb2 branch
>>> instruction, but there's a change to how fixups are handled which is
>>> larger than we're comfortable with committing blind and that test is
>>> an order of magnitude longer than most (any alternative to .space to
>>> make llvm-mc think a label is PC-relative and far away without
>>> actually generating the intervening gap?)
>>>
>>> Cheers.
>>>
>>> Tim.
>>>
>>> (Attached file is identical to Chris's original).
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>