Page MenuHomePhabricator

[Clang][RISCV][RFC] Add byval parameter attribute?
Needs ReviewPublic

Authored by luismarques on Mar 3 2021, 5:24 PM.

Details

Reviewers
asb
mundaym
Summary

Should we add the byval parameter attribute to struct parameters?

Pros:

  • Improves optimizations;
  • Used by MSan instrumentation pass (without it a test will fail).

Cons:

  • ?

An example of an optimization without byval and with byval:

$ cat test.c
struct S { char c[32]; };
void foo(struct S s) {
    s.c[0] = 42;
}
$ clang --target=riscv64-linux -O2 -S -o- test.c

Without byval:

foo:
        addi    a1, zero, 42
        sb      a1, 0(a0)
        ret

With byval:

foo:
        ret

I'm not sure what the impact would be of also adding the attribute to vector parameters, so this patch doesn't do so.

Diff Detail

Event Timeline

luismarques created this revision.Mar 3 2021, 5:24 PM
luismarques requested review of this revision.Mar 3 2021, 5:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 3 2021, 5:24 PM
asb added a comment.EditedMar 4 2021, 5:06 AM

I think I may have had the impression from some previous discussions that byval may have limited positive impact, and that letting Clang add the copies to the IR might in some cases help optimisations (that may not be written to reason about byval). You've got a good example of a case where the lack of byval causes weaker optimisation though.

Just running this across the GCC torture suite on rv32imafdc_{ilp32,ilp32d}, rv64imafdc_{lp64, lp64d} and {O0,O1,O2,O3,Os}, I see 22394 insertions(+), 17462 deletions(-) in the generated .s. Obviously not a representative benchmark, but it does seem there are potential regressions to consider.