~mpu/qbe

3

WARNING: if-optimisation patch breaks hare master branch

Details
Message ID
<CAAS8gYBG=N4EqBs50Y4m_TojRRxdDaGYoVZ0kD07m7cewLFK5w@mail.gmail.com>
DKIM signature
pass
Download raw message
Sorry for the new topic, but just for visibility.

Pulled the latest hare master
(74719e1e76ba307582a3f7cbaa8dddfb316bcb19) and am getting:

~/src/hare$ make clean; make check
...
qbe:.cache/bytes.ssa:2402: ssa temporary %ifc.1135 is used undefined
in @matches.995

Clearly an if-conversion snafu cos of the temporary name "%ifc.*".

Ooops, and will investigate...

R
Details
Message ID
<CAAS8gYA2+gW-+HASTgOMGP99Hx_jmm=66CgCwDoxy3+B=kaaCQ@mail.gmail.com>
In-Reply-To
<CAAS8gYBG=N4EqBs50Y4m_TojRRxdDaGYoVZ0kD07m7cewLFK5w@mail.gmail.com> (view parent)
DKIM signature
pass
Download raw message
> qbe:.cache/bytes.ssa:2402: ssa temporary %ifc.1135 is used undefined
> in @matches.995

Gack, I see what's happening.

The current code attempts to re-use mask temporaries created by
previous if-conversions.

However, this is only valid if the new if-conversion is in a block
dominated by the earlier if-conversion.

Doh!

In the positive side it looks like if-conversion is finding plenty of
opportunity in the hare code-base, including mask re-use:

> If-conversion:
                                $bytes.peek_token If-converting @.897
- true-pred @true.917 false-pred @false.918 join @.919 - 1 phis
                                $bytes.peek_token If-converting @.919
- true-pred @.927 false-pred @.919 join @.928 - 1 phis
                                $bytes.peek_token If-converting @.924
- true-pred @.935 false-pred @.924 join @.936 - 1 phis
                                $bytes.peek_token If-converting
@true.945 - true-pred @true.955 false-pred @false.956 join @.957 - 1
phis
                                $bytes.peek_token If-converting
@matches.1045 - true-pred @.1058 false-pred @matches.1045 join @.1059
- 1 phis
                                $bytes.peek_token If-converting
@false.1055 - true-pred @.1076 false-pred @false.1055 join @.1077 - 1
phis
                                $bytes.peek_token If-converting @.1077
- true-pred @true.1072 false-pred @false.1073 join @.1074 - 1 phis
                                $bytes.peek_token If-converting
@matches.995 - true-pred @.1011 false-pred @matches.995 join @.1012 -
1 phis
                                $bytes.peek_token If-converting
@false.1008 - true-pred @.1031 false-pred @false.1008 join @.1032 - 1
phis
                                $bytes.peek_token If-converting @.1032
- true-pred @true.1027 false-pred @false.1028 join @.1029 - 2 phis

> After if-conversion:

...
@.897
    dbgloc 96, 14
    dbgloc 96, 32
    dbgloc 96, 27
    dbgloc 96, 27
    %field.913 =l add %s, 48
    %.914 =l load %field.913
    dbgloc 96, 32
    %.911 =w csltl %.914, 0
    dbgloc 97, 14
    dbgloc 97, 25
    dbgloc 97, 34
    dbgloc 97, 37
    dbgloc 97, 50
    %ifc.1128 =l extuw %.911
    %ifc.1129 =l neg %ifc.1128    <--- true mask
    %ifc.1130 =l sub %ifc.1128, 1 <--- false mask
    %ifc.1131 =l and %ifc.1130, $bytes.index
    %ifc.1132 =l and %ifc.1129, $bytes.rindex
    %.916.985 =l or %ifc.1131, %ifc.1132
@.919
    dbgloc 99, 14
    dbgloc 100, 52
    dbgloc 99, 57
    dbgloc 99, 35
    dbgloc 99, 57
    dbgloc 99, 38
    dbgloc 99, 38
    %.930 =w cnel %.914, -9223372036854775808
    %ifc.1133 =w and %ifc.1130, %.911   <--- (valid) mask re-use
    %ifc.1134 =w and %ifc.1129, %.930
    %.926.988 =w or %ifc.1133, %ifc.1134
@.928
...

Will fix real soon now...

R
Details
Message ID
<CAAS8gYBk5Z0fq__NoqGKm4GJTcuXVdHAtxeoNXAWqx4mmcvWaQ@mail.gmail.com>
In-Reply-To
<CAAS8gYA2+gW-+HASTgOMGP99Hx_jmm=66CgCwDoxy3+B=kaaCQ@mail.gmail.com> (view parent)
DKIM signature
pass
Download raw message
Fix:

---

diff --git a/all.h b/all.h
index 39f3701..47d41f4 100644
--- a/all.h
+++ b/all.h
@@ -353,6 +353,7 @@ struct Tmp {
        } width;
        int visit;
        uint gcmbid;
+       uint ifcbid;
        int ifckl;
        Ref ifct;
        Ref ifcf;
diff --git a/ifopt.c b/ifopt.c
index 1f13295..bf5ecc2 100644
--- a/ifopt.c
+++ b/ifopt.c
@@ -253,7 +253,7 @@ setdef(Fn *fn, uint bid, Ref r, Ins *i)
 }

 /* If-convert small if-then[-else] using bitmasks. */
-/* needs rpo pred use; breaks cfg use */
+/* needs rpo pred use dom; breaks cfg use */
 void
 ifconvert(Fn *fn)
 {
@@ -314,7 +314,7 @@ ifconvert(Fn *fn)
                }
                assert(rtype(b->jmp.arg) == RTmp);
                t = &fn->tmp[b->jmp.arg.val];
-               if (nphis != 0 && (req(t->ifct, R) || (needkl && !t->ifckl))) {
+               if (nphis != 0 && (req(t->ifct, R) || (needkl &&
!t->ifckl) || !dom(fn->rpo[t->ifcbid], b))) {
                        /* extend boolean val to Kl - not always necessary */
                        if (needkl) {
                                boolv = newtmp("ifc", Kl, fn);
@@ -330,6 +330,7 @@ ifconvert(Fn *fn)
                        ins[n++] = (Ins) {.op = Osub, .cls = (needkl ?
Kl : Kw), .to = maskf, .arg = {boolv, con01[1]}};
                        setdef(fn, bid, maskf, &ins[n-1]);
                        t = &fn->tmp[b->jmp.arg.val]; /* might have moved */
+                       t->ifcbid = bid;
                        t->ifckl = needkl;
                        t->ifct = maskt;
                        t->ifcf = maskf;
Details
Message ID
<CAAS8gYBcrB==d7M1kZ4b7mtRLaQgFPDEhA97WZ02HznMyUBLWA@mail.gmail.com>
In-Reply-To
<CAAS8gYA2+gW-+HASTgOMGP99Hx_jmm=66CgCwDoxy3+B=kaaCQ@mail.gmail.com> (view parent)
DKIM signature
pass
Download raw message
Another observation from this test case - see below code snippet - is
that QBE code generated by hare frequently uses jnz+phi's to combine
cmp (boolean) conditions.

Quentin and I noticed cproc doing something similar and after some
attempts to "fix this" in QBE, Quentin came up with a neat
cproc patch - https://lists.sr.ht/~mcf/cproc/patches/53742 - instead.

The general-purpose if-conversion in this patch can be simplified for
boolean-only values (and/or condition combining), and/but there (also)
might be better ways in general of wrangling either the hare code-gen
or early QBE CFG manipulation.

> @.897
>     dbgloc 96, 14
>     dbgloc 96, 32
>     dbgloc 96, 27
>     dbgloc 96, 27
>     %field.913 =l add %s, 48
>     %.914 =l load %field.913
>     dbgloc 96, 32
>     %.911 =w csltl %.914, 0
>     dbgloc 97, 14
>     dbgloc 97, 25
>     dbgloc 97, 34
>     dbgloc 97, 37
>     dbgloc 97, 50
>     %ifc.1128 =l extuw %.911
>     %ifc.1129 =l neg %ifc.1128    <--- true mask
>     %ifc.1130 =l sub %ifc.1128, 1 <--- false mask
>     %ifc.1131 =l and %ifc.1130, $bytes.index
>     %ifc.1132 =l and %ifc.1129, $bytes.rindex
>     %.916.985 =l or %ifc.1131, %ifc.1132
> @.919
>     dbgloc 99, 14
>     dbgloc 100, 52
>     dbgloc 99, 57
>     dbgloc 99, 35
>     dbgloc 99, 57
>     dbgloc 99, 38
>     dbgloc 99, 38
>     %.930 =w cnel %.914, -9223372036854775808
>     %ifc.1133 =w and %ifc.1130, %.911   <--- %.911 =w csltl %.914, 0
>     %ifc.1134 =w and %ifc.1129, %.930   <--- %.930 =w cnel %.914, -9223372036854775808
>     %.926.988 =w or %ifc.1133, %ifc.1134
> @.928
> ...
>
> Will fix real soon now...
>
> R
Reply to thread Export thread (mbox)