output float immediate value with %f instead of %g

Submitted by Guo Yejun on July 30, 2015, 7:34 a.m.

Details

Message ID 854E8DBA9F41904AB047E03BB6963AE505576CCB@SHSMSX101.ccr.corp.intel.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Guo Yejun July 30, 2015, 7:34 a.m.
Ping for review, thanks.

-----Original Message-----
From: Guo, Yejun 
Sent: Thursday, July 23, 2015 8:48 AM
To: beignet@lists.freedesktop.org
Cc: Guo, Yejun
Subject: [PATCH] output float immediate value with %f instead of %g

In disassemble, a float immediate value is printf with %g, it outputs a wrong value with 8 bytes, change it to be %f with the correct 4 bytes. At some cases, the real data type is not float, so also print the value in hex format.

Signed-off-by: Guo Yejun <yejun.guo@intel.com>
---
 backend/src/backend/gen/gen_mesa_disasm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--
1.9.1

Patch hide | download patch | download mbox

diff --git a/backend/src/backend/gen/gen_mesa_disasm.c b/backend/src/backend/gen/gen_mesa_disasm.c
index df6156c..090143e 100644
--- a/backend/src/backend/gen/gen_mesa_disasm.c
+++ b/backend/src/backend/gen/gen_mesa_disasm.c
@@ -986,7 +986,7 @@  static int imm(FILE *file, uint32_t type, const void* inst)
       format(file, "0x%xV", GEN_BITS_FIELD(inst, bits3.ud));
       break;
     case GEN_TYPE_F:
-      format(file, "%-gF", GEN_BITS_FIELD(inst, bits3.f));
+      format(file, "%.*fF (0x%x)", GEN_BITS_FIELD(inst, bits3.f), 
+ GEN_BITS_FIELD(inst, bits3.ud));
       break;
     case GEN_TYPE_UL:
       assert(!(gen_version < 80));

Comments

> 

> In disassemble, a float immediate value is printf with %g, it outputs a wrong

> value with 8 bytes, change it to be %f with the correct 4 bytes. At some cases,

> the real data type is not float, so also print the value in hex format.


As far as I know, %g will select %f or %e automatically, which is good for formatting.
The compiler would convert the float to double, then printf will output the double (8 bytes) value.
I am not quite sure on this. So I don't think this will bring in any issue. What kind of problem do you meet?

I agree with output the hex value at first glance of the patch, but the output asm would looks like
a little messy, so I don't think it is a good idea to output the hex value.
    (    1308)  mul(16)         g110<1>:F       g112<8,8,1>:F   1e-30F (0xda24260) { align1 WE_normal 1H };

Thanks!
Ruiling
For the kernel

uint aaa = 0xfffefffe;
...

It is finally compiled into:

(      12)  mov(1)          g127.7<1>:F     6.90887e-310F                   { align1 WE_all};


I debugged into the function and found that, %g outputs 8 bytes (4 bytes for aaa and 4 bytes following aaa with random value). And, to get the exact value of aaa (with real format uint), the hex format is a necessary.

-----Original Message----- 
From: Song, Ruiling 

Sent: Friday, July 31, 2015 4:02 PM
To: Guo, Yejun; beignet@lists.freedesktop.org
Subject: RE: [Beignet] [PATCH] output float immediate value with %f instead of %g

> 

> In disassemble, a float immediate value is printf with %g, it outputs 

> a wrong value with 8 bytes, change it to be %f with the correct 4 

> bytes. At some cases, the real data type is not float, so also print the value in hex format.


As far as I know, %g will select %f or %e automatically, which is good for formatting.
The compiler would convert the float to double, then printf will output the double (8 bytes) value.
I am not quite sure on this. So I don't think this will bring in any issue. What kind of problem do you meet?

I agree with output the hex value at first glance of the patch, but the output asm would looks like a little messy, so I don't think it is a good idea to output the hex value.
    (    1308)  mul(16)         g110<1>:F       g112<8,8,1>:F   1e-30F (0xda24260) { align1 WE_normal 1H };

Thanks!
Ruiling
> -----Original Message-----

> From: Guo, Yejun

> Sent: Sunday, August 2, 2015 10:42 AM

> To: Song, Ruiling; beignet@lists.freedesktop.org

> Subject: RE: [Beignet] [PATCH] output float immediate value with %f instead

> of %g

> 

> For the kernel

> 

> uint aaa = 0xfffefffe;

> ...

> 

> It is finally compiled into:

> 

> (      12)  mov(1)          g127.7<1>:F     6.90887e-310F                   { align1 WE_all};


I would rather change this instruction to UD data type in our backend:
(      12)  mov(1)          g127.7<1>:UD     0xfffeffe                   { align1 WE_all};
This would make things a little clearer. I am not sure anybody has different opinion?

Thanks!
Ruiling
> 

> 

> I debugged into the function and found that, %g outputs 8 bytes (4 bytes for aaa

> and 4 bytes following aaa with random value). And, to get the exact value of aaa

> (with real format uint), the hex format is a necessary.
Yes, it would be better, not sure why the current code generates "F" for the mov instruction, I'll take a look.

-----Original Message-----
From: Song, Ruiling 

Sent: Tuesday, August 04, 2015 10:45 AM
To: Guo, Yejun; beignet@lists.freedesktop.org
Subject: RE: [Beignet] [PATCH] output float immediate value with %f instead of %g



> -----Original Message-----

> From: Guo, Yejun

> Sent: Sunday, August 2, 2015 10:42 AM

> To: Song, Ruiling; beignet@lists.freedesktop.org

> Subject: RE: [Beignet] [PATCH] output float immediate value with %f 

> instead of %g

> 

> For the kernel

> 

> uint aaa = 0xfffefffe;

> ...

> 

> It is finally compiled into:

> 

> (      12)  mov(1)          g127.7<1>:F     6.90887e-310F                   { align1 WE_all};


I would rather change this instruction to UD data type in our backend:
(      12)  mov(1)          g127.7<1>:UD     0xfffeffe                   { align1 WE_all};
This would make things a little clearer. I am not sure anybody has different opinion?

Thanks!
Ruiling
> 

> 

> I debugged into the function and found that, %g outputs 8 bytes (4 

> bytes for aaa and 4 bytes following aaa with random value). And, to 

> get the exact value of aaa (with real format uint), the hex format is a necessary.
> -----Original Message-----

> From: Guo, Yejun

> Sent: Tuesday, August 4, 2015 10:50 AM

> To: Song, Ruiling; beignet@lists.freedesktop.org

> Subject: RE: [Beignet] [PATCH] output float immediate value with %f instead

> of %g

> 

> Yes, it would be better, not sure why the current code generates "F" for the mov

> instruction, I'll take a look.

The code generator select float type instead of integer type because IVB has more floating point ALU over integer ones.
See LoadImm pattern in gen_insn_selection.cpp
But I don't think this is a performance sensitive point.

Thanks!
Ruiling
Got it, thanks.

The ASM is outputted for human reading, it is expected that a correct and human readable format is used.

-----Original Message-----
From: Song, Ruiling 

Sent: Tuesday, August 04, 2015 12:41 PM
To: Guo, Yejun; beignet@lists.freedesktop.org
Subject: RE: [Beignet] [PATCH] output float immediate value with %f instead of %g



> -----Original Message-----

> From: Guo, Yejun

> Sent: Tuesday, August 4, 2015 10:50 AM

> To: Song, Ruiling; beignet@lists.freedesktop.org

> Subject: RE: [Beignet] [PATCH] output float immediate value with %f 

> instead of %g

> 

> Yes, it would be better, not sure why the current code generates "F" 

> for the mov instruction, I'll take a look.

The code generator select float type instead of integer type because IVB has more floating point ALU over integer ones.
See LoadImm pattern in gen_insn_selection.cpp But I don't think this is a performance sensitive point.

Thanks!
Ruiling