[llvm-commits] [PATCH] thumb t2b fix

[Owen Anderson]

Greg,

Unless it's really impossible to make it work like this, I think the correct solution is to have the value of the MCOperand be S:J1:J2:imm10:imm11.  The .td file then just needs to splat the bits into the correct locations, and it's the responsibility of the creator of the MCInst to pre-encode the offset immediate properly.

--Owen

On Sep 18, 2012, at 6:05 PM, Greg Fitzgerald <garious at gmail.com> wrote:

> 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
>>