RE: sid, base_insn vs entire_insn and ambiguity

[Aurélien Buhrig <aurelien dot buhrig at tiempo-ic dot com>]

It's what I did before I change base insn from 16 to 32 to solve decoding
problem in sid, and it worked.
Now base_insn is 32 the ambiguity occurs... and I am forced to set derived
operand with ifield-assertion to solve this ambiguity.

Looking deeper in the code, it seems the bug comes from -sub-insn-make!
function (insn.scm, at the FIXME line ;-).
Indeed, when instantiating multi-insn, ifield assertion is set with the the
ifield-assertion of the multi-insn that is has not been set yet (so #f). I
think it should be a andif between all the ifield-assertion of the
derived-operands with which the current instruction is instantiated.

But now the problem has moved elsewhere... I suppose setting something else
that #f is not yet supported... Any help is welcome!

Aurélien


-----Message d'origine-----
De?: cgen-owner@sourceware.org [mailto:cgen-owner@sourceware.org] De la part
de Dave Brolley
Envoyé?: vendredi 27 juin 2008 18:04
À?: Aurélien Buhrig
Cc?: cgen@sourceware.org
Objet?: Re: sid, base_insn vs entire_insn and ambiguity

This may be a case that is not suitable for derived operands. The field 
in questions appears to be a register number for add-cst-binop-pd and a 
fixed value of 0x7 for add-cst-binop-aad. That suggests to me that you 
may want to define two separate insns, since this is not really a case 
of different addressing modes for one particular operand.

 (dni
      (name add-pd)
      ("add cst -> aadpd")
      ()
      ("add.$len $scst,${dreg}")
      (+ len SEL_2OP BINOP_ADD scst dreg)
      (nop)
      ())

 (dni
      (name add-aad)
      ("add cst -> aadpd")
      ()
      ("add.$len $scst,${abs16}")
      (+ len SEL_2OP BINOP_ADD scst (f-dreg 7) abs16)
      (nop)
      ())

You will need to drop PC from enum-dreg in order to prevent it from 
being accepted in the syntax for add-pd. If the reg operand/ifield/enum 
is used in other insns where PC is a valid choice, then you will need to 
define a new operand/ifield/enum for this insn.



Aurélien Buhrig wrote:
> I do have ifield-assertion on conflicting operand.
> (one is set to (eq dreg 7) and the other to (ne dreg 7) )
> 	
> So it seem some fields are not correctly set or are lost somewhere...
>
> I cannot see the generated code since the generation is aborted before
> printing the decode function.
>
> How can I debug or help debugging ?
>
> Aurelien
>
>
> -----Message d'origine-----
> De : Dave Brolley [mailto:brolley@redhat.com] 
> Envoyé : jeudi 26 juin 2008 18:17
> À : Aurélien Buhrig
> Cc : cgen@sourceware.org
> Objet : Re: sid, base_insn vs entire_insn and ambiguity
>
> If you got the message "Unable to resolve ambiguity", then at least one 
> of the conflicting insns did not have an ifield assertion. Otherwise 
> CGEN should go on to generate the decoder switch entry. Did you check 
> the generated code to see if that happened? Looking at the 
> insn-base-mask for add-cst-binop-pd, I suspect that it may not have an 
> ifield assertion for bits 13-15.
>
> Dave
>
> Aurélien Buhrig wrote:
>   
>> Humm... it seems the problem was not fixed...
>> Don't know why I has passed once, but after a whole cleanup of the build
>> directory, I cannot get rid of the ambiguity error.
>>
>> In interactive mode, the output is :
>> --------------------------------------------------
>> Building subtable at index 1027, decode-bitsize = 16, indices used thus
>>     
> far:
>   
>> 1 2 3 4 5 7 8 9 10 11 12
>> Best decode bits (prev=(1 2 3 4 5 7 8 9 10 11 12 ) start=0
>>     
> decode=16)=>((13
>   
>> 14 15 ))
>> insn =add-cst-binop-pd insn-value=4018 insn-base-mask=7c78 insn-len=16
>> decode-len=3 opcode=0 opcode-mask=0 indices=(0 4 2 6 1 5 3 7 )
>> insn =add-cst-binop-aad insn-value=401f0000 insn-base-mask=7c7f0000
>> insn-len=32 decode-len=3 opcode=7 opcode-mask=7 indices=(0 )
>> Processing decoder for bits 13 14 15 ...
>> Processing decode entry 0 in decode_table_1027_0, add.${len}
>> ${scst},-($dreg) ...
>> Processing decode entry 1 in decode_table_1027_1, add.${len}
>> ${scst},-($dreg) ...
>> Processing decode entry 2 in decode_table_1027_2, add.${len}
>> ${scst},-($dreg) ...
>> Processing decode entry 3 in decode_table_1027_3, add.${len}
>> ${scst},-($dreg) ...
>> Processing decode entry 4 in decode_table_1027_4, add.${len}
>> ${scst},-($dreg) ...
>> Processing decode entry 5 in decode_table_1027_5, add.${len}
>> ${scst},-($dreg) ...
>> Processing decode entry 6 in decode_table_1027_6, add.${len}
>> ${scst},-($dreg) ...
>> Processing decode entry 7 in decode_table_1027_7, subtable ...
>> Building subtable at index 7, decode-bitsize = 16, indices used thus far:
>>     
> 13
>   
>> 14 15 1 2 3 4 5 7 8 9 10 11 12
>> -population-top-few: count-threshold is zero!
>> Best decode bits (prev=(13 14 15 1 2 3 4 5 7 8 9 10 11 12 ) start=0
>> decode=16)=>(())
>> -population-top-few: count-threshold is zero!
>> Best decode bits (prev=(13 14 15 1 2 3 4 5 7 8 9 10 11 12 ) start=16
>> decode=0)=>(())
>> Filtering 2 instructions for non specializations.
>> WARNING: Decoder ambiguity detected:  add-cst-binop-aad, add-cst-binop-pd
>> Filtering 2 instructions for identical variants.
>> -----------------------------------------------------
>>
>> Since applying add-cst-binop-pd mask on both add-cst-binop-pd and
>> add-cst-binop-add insn gives the same result, I understand there is an
>> ambiguity.
>>
>>
>> But looking a bit deeper into the code (decode.scm), there is this
comment
>>     
> :
>   
>>   ; FIXME: For now we assume that if they all have an
>>   ; ifield-assertion spec, then there is no ambiguity (it's left
>>   ; to the programmer to get it right).  This can be made more
>>   ; clever later.
>>
>> Since there IS ifield-assertion, why this ambiguity is not resolved ???
>> I can print backtrace if it helps...
>>
>> Thanks
>> Aurélien
>>
>>
>> -----Message d'origine-----
>> De : cgen-owner@sourceware.org [mailto:cgen-owner@sourceware.org] De la
>>     
> part
>   
>> de Aurélien Buhrig
>> Envoyé : jeudi 26 juin 2008 11:26
>> À : cgen@sourceware.org
>> Objet : RE: sid, base_insn vs entire_insn and ambiguity
>>
>> It seems I solved the problem replacing (not (eq ..)) by (ne ...) in the
>> ifield-assertion.
>> Sorry about that!
>>
>> Aurélien
>>
>>
>> -----Message d'origine-----
>> De : cgen-owner@sourceware.org [mailto:cgen-owner@sourceware.org] De la
>>     
> part
>   
>> de Aurélien Buhrig
>> Envoyé : mercredi 25 juin 2008 09:36
>> À : 'Dave Brolley'
>> Cc : cgen@sourceware.org
>> Objet : RE: sid, base_insn vs entire_insn and ambiguity
>>
>> Hi,
>>
>> I succeed in solving the disassembly problem by using "buf" instead of
>> "value" in CGEN_DIS_HASH macro...
>> For the decode, with "base-insn-bitsize 32", the decoding is ok. 
>>
>> The only pb is the ambiguity. This ambiguity error only occurs with
>> "base-insn-bitsize 32". With "base-insn-bitsize 16", there is no warning.
>>
>> Let's take an example.
>>
>> add.w csti, @abs16 is encoded the same way than add.w csti, -(rj), but
>>     
> with
>   
>> rj=PC and with a second word (the abs16 address) that follows this
>> instruction, (PC is not accessible in the assembly syntax, so add.w csti,
>> -(PC) is forbidden)
>>
>> -------------------------------------------------------------
>> ; byte/word (bit 0)
>> (dnf    f-bw        "bit/word selector"             ()  0   1)
>> ; 2 operands instruction opcode (bits 2-5)
>> (dnf    f-2opopc        "2 operand opcode"          ()  2   4)
>> ; Source register (bits 6-8)
>> (dnf    f-sreg          "Source register"           ()  6   3)
>> ; 2op addressing mode field (bits 9-12)
>> (dnf    f-ad30          "ad[3-0]"                   ()  9   4)
>> ; Destination register (bits 13-15) 
>> (dnf    f-dreg          "Destination register"      ()  13  3)
>> (define-ifield
>>    (name f-abs16)
>>    (comment "")
>>    (attrs (ABS-ADDR))
>>    (word-offset 16)
>>    (word-length 16)
>>    (start 0)
>>    (length 16)
>>    (mode UINT)
>>    (encode #f)
>>    (decode #f)))
>>
>> (define-normal-insn-enum
>>   enum-sel1  "bit 1 sel"     ()   SEL_  f-sel1
>>   ( ("2OP" 1) ))
>> (define-normal-insn-enum
>>   enum-2opopc  "2 Operand Opcodes"  ()   BINOP_   f-2opopc
>>   ( ("ADD"  0) ("ADDC" 1)  ("SUB"  2)   ("SUBC" 3)
>>    ("CMP"  4)  ("TB"  5)  ("CB"  6)   ("SB"  7)   ("OR"  7)
>>    ("AND"  8)  ("XOR"  9)  ("MOVEP" 10)  ("MOVE" 11) ))
>>
>> (define-normal-insn-enum
>>   enum-sreg  "sreg enums"    ()   SREG_    f-sreg 
>>   ( ("R0" 0) ("R1"   1) ("R2"   2) ("R3"   3)
>>  ("R4"   4) ("R5"   5) ("SP" 6) ("PC" 7) ))
>>
>> (define-normal-insn-enum
>>   enum-ad30  "2op addressing mode"  ()   BINAD_   f-ad30
>>   ( ("CSTIND"  0) ("CSTDRN"  1) ("CSTPI"   2) ("CSTPD"   3) ("CSTAAD"
3)
>>    ("RNIND"  4) ("RNDRN"   5) ("RNPI"   6) ("RNPD"   7) ("RNAAD"   7)
>>    ("PDRN"   8) ("PIRN"   9) ("DRNRN" 10) ("INDRN"  11) ("AADRN"  12)
>>  ("RNRN"  13) ("CSTRN" 14) ))
>>
>> (define-normal-insn-enum
>>   enum-dreg  "dreg enums"    ()   DREG_    f-dreg
>>   ( ("R0" 0) ("R1"   1) ("R2"   2) ("R3"   3)
>>  ("R4"   4) ("R5"   5) ("SP" 6) ("PC" 7) ))
>>
>> (define-hardware
>>     (name       h-gr)
>>     (comment    "General registers")
>>     (type       register HI(7))
>>     (indices 	keyword "" ((sp 6)(r0 0) (r1 1) (r2 2) (r3 3) (r4 4)
(r5 5)
>> (r6 6)))
>>     (get        (index) (c-call HI "@cpu@_h_gr_get_handler" index))
>>     (set        (index newval) (c-call VOID "@cpu@_h_gr_set_handler"
index
>> newval)))
>> (define-hardware
>>     (name       h-len)
>>     (comment    "Instruction length")
>>     (type       register BI (2))
>>     (indices    keyword "" ((b 0) (w 1)))
>>     (get        (index) (c-call HI "@cpu@_h_len_get_handler" index)))
>>
>> (dnop abs16 "16 bit absolute address" (RELAX) h-iaddr f-abs16)
>> (dnop dreg "Destination register" () h-gr f-dreg)
>> (dnop scst "Source Constant" ()        h-cst   f-sreg)
>> (dnop len "instr len" () h-len   f-bw)
>>
>> (define-derived-operand 
>>   (name binop-aad) 
>>   (comment "") 
>>   (attrs) 
>>   (mode UHI) 
>>   (args (abs16)) 
>>   (syntax "$abs16") 
>>   (base-ifield f-2opopc) 
>>   (encoding (+ BINAD_CSTAAD DREG_PC abs16 ) ) 
>>   (ifield-assertion (eq f-dreg 7)) 
>>   (getter (nop)) 
>>   (setter (nop)))
>>
>>  (define-derived-operand 
>>   (name binop-pd) 
>>   (comment "") 
>>   (attrs) 
>>   (mode UHI) 
>>   (args (dreg)) 
>>   (syntax "$-(dreg)") 
>>   (base-ifield f-2opopc) 
>>   (encoding (+ BINAD_CSTPD dreg ) ) 
>>   (ifield-assertion (not (eq f-dreg 7))) 
>>   (getter (nop)) 
>>   (setter (nop))) 
>>  
>>
>> (define-anyof-operand                    
>>   (name binop-aad-pd) 
>>   (comment "PD addressing modes") 
>>   (mode UHI) 
>>   (base-ifield f-2opopc) 
>>   (choices binop-aad binop-pd))
>>
>>
>>   (dni
>>       (name add-cst)
>>       ("add cst -> aadpd")
>>       ()
>>       ("add.$len $scst,${binop-aad-pd}")
>>       (+ len SEL_2OP BINOP_ADD scst binop-aad-pd)
>>       (nop)
>>       ())
>>
>> ------------------------------------
>>
>>
>> So the instruction are encoded like that :
>> (+ len 1 0000 scst 0011 dreg) for add.x csti, -(rn) ----- (dreg != 7)
>> (+ len 1 0000 scst 0011 111 abs16) for add.x csti, -(rn)
>>
>> I would like the ifield-assertion differentiates the two instructions,
but
>> it fails.
>>
>> Thanks for your help!
>> Aurélien
>>
>>
>>
>>
>>
>>
>> -----Message d'origine-----
>> De : Dave Brolley [mailto:brolley@redhat.com] 
>> Envoyé : mardi 24 juin 2008 17:38
>> À : Aurélien Buhrig
>> Objet : Re: sid, base_insn vs entire_insn and ambiguity
>>
>> Hi,
>>
>> It's hard to help with this without more complete information. You don't 
>> provide the declarations for your ifields or insn enumerations, which 
>> could also be the source of the ambiguity.
>>
>> Aurélien Buhrig wrote:
>>   
>>     
>>>  I tried
>>> the second solution that consists in setting the base-insn-bitsize to
32.
>>>   
>>>     
>>>       
>> Did you also do the second part of the solution which was to write the 
>> necessary code in the decoder (<arch>::step_insns in 
>> sid/component/cgen-cpu/<arch>/<arch>.cxx) to make sure that both 16 bit 
>> and 32 bit insns are aligned in base_insn and entire_insn? i.e., since 
>> you don't know whether each insn will be 16 or 32 bits, you need to 
>> always read 32 bits:
>>
>>           // Fetch 16-bit pieces separately, so endianness
>>           // conversions can be done on this chunk size.
>>          UHI insn0 = this->GETIMEMHI (pc, pc);
>>          UHI insn1 = this->GETIMEMHI (pc, pc+2);
>>          USI insn = (insn0 << 16) | insn1;
>>          sem->decode (this, pc, insn, insn);
>>   
>>     
>>> I have now 2 problems : 
>>> - The disassembly is not correct anymore.
>>> - I have an unresolved ambiguity error.
>>> ERROR: Unable to resolve ambiguity (maybe need some ifield-assertion
>>>     
>>>       
>> specs?)
>>   
>>     
>>> So I tried derived operand with ifield-assertion, the I cannot fix the
>>> ambiguity problem.
>>> Here is the description of the faulty insn:
>>>   
>>>     
>>>       
>> It's hard to see what the problem here might be without seeing your 
>> ifield and insn enum specs.
>>   
>>     
>>> (define-derived-operand  
>>>   (name unop-pi-pc-imm52)
>>>   (comment "")  
>>>   (attrs)
>>>   (mode UHI) 
>>>   (args (uimm52) (reg))
>>>   (syntax "$uimm52")
>>>   (base-ifield f-1opopc)
>>>   (encoding (+ UNAD_PI REG_PC uimm52 ) )
>>>   (ifield-assertion (eq f-reg 7))
>>>   (getter (nop))
>>>   (setter (nop)))
>>>
>>> (define-derived-operand
>>>   (name unop-pi-gr)
>>>   (comment "")
>>>   (attrs)
>>>   (mode UHI)
>>>   (args (reg))
>>>   (syntax "$(reg)+")
>>>   (base-ifield f-1opopc)
>>>   (encoding (+ UNAD_PI reg ) )
>>>   (ifield-assertion (not (eq f-reg 7)))
>>>   (getter (nop))
>>>   (setter (nop)))
>>>
>>> (define-anyof-operand
>>>   (name unop-pi)
>>>   (comment "PI addressing modes")
>>>   (mode UHI)
>>>   (base-ifield f-1opopc)
>>>   (choices unop-pi-pc-imm52 unop-pi-gr))
>>>
>>> (dni
>>>       moveps                                                ; name
>>>       (.str mnemonic " (rn)+")                                  ;
comment
>>>       ()                                                        ; attrs
>>>       ("move.$len $sr,($reg)+")                                      ;
>>> syntax
>>>       ;+ (+ len SEL_1OP ONEOP_MTSR UNAD_PI reg)             ; format
>>>       (+ len SEL_1OP ONEOP_MTSR unop-pi)                ; format
>>>       (nop)
>>>       ())     
>>>
>>>
>>> How can I do ?
>>> Thanks,
>>> Aurelien
>>>
>>>   
>>>     
>>>       
>>   
>>     
>
>