This is an archive of the discontinued LLVM Phabricator instance.

[LoongArch] Explicitly specify instruction properties
ClosedPublic

Authored by hev on Jun 30 2023, 2:11 AM.

Details

Summary

This revision explicitly specifies the machine instruction properties instead of relying on guesswork. This is because guessing instruction properties has proven to be inaccurate, such as the machine LICM not working:

void func(char *a, char *b)
{
    int i;

    for (i = 0; i != 72526; i++)
        a[i] = b[i];
}

Guessing instruction properties:

func:                                   # @func
# %bb.0:
        move    $a2, $zero
.LBB0_1:                                # =>This Inner Loop Header: Depth=1
        ldx.b   $a3, $a1, $a2
        stx.b   $a3, $a0, $a2
        addi.d  $a2, $a2, 1
        lu12i.w $a3, 17
        ori     $a3, $a3, 2894
        bne     $a2, $a3, .LBB0_1
# %bb.2:
        ret
.Lfunc_end0:

Explicitly specify instruction properties:

func:                                   # @func
# %bb.0:
        lu12i.w $a2, 17
        ori     $a2, $a2, 2894
        move    $a3, $zero
.LBB0_1:                                # =>This Inner Loop Header: Depth=1
        ldx.b   $a4, $a1, $a3
        stx.b   $a4, $a0, $a3
        addi.d  $a3, $a3, 1
        bne     $a3, $a2, .LBB0_1
# %bb.2:
        ret
.Lfunc_end0:

Diff Detail

Event Timeline

hev created this revision.Jun 30 2023, 2:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 30 2023, 2:11 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
hev requested review of this revision.Jun 30 2023, 2:11 AM
xen0n added a comment.Jun 30 2023, 2:49 AM

The diff is missing context lines, I guess you created the patch with plain git diff and posted the diff with the web interface... Checkout the recommendations or set up Arcanist and use arc diff. (In case of Arcanist you may have to apply some PHP 8 fixes but it should be straight-forward otherwise.)

llvm/lib/Target/LoongArch/LoongArchInstrInfo.td
779

I checked PPC and RISCV and it seems prefetch instructions should have mayLoad = 1, mayStore = 1. Otherwise I expect them to get incorrectly optimized away...

1915

These instructions should probably be hasSideEffects = 1 because they instruct the hardware to perform certain actions. Otherwise they will probably get deleted too given they don't return anything...

hev updated this revision to Diff 536162.Jun 30 2023, 3:08 AM
hev marked 2 inline comments as done.

Thanks for your comments.

xen0n accepted this revision.Jun 30 2023, 5:48 AM

LGTM; you may want to coordinate with the author of D154183 and D154195 so no instruction will miss any marker.

This revision is now accepted and ready to land.Jun 30 2023, 5:48 AM
SixWeining accepted this revision.Jul 3 2023, 12:41 AM

LGTM. Thanks!

This revision was automatically updated to reflect the committed changes.