[PATCH] D148269: [Pipeline] Don't limit ArgumentPromotion to -O3

[Yonghong Song via Phabricator via llvm-commits]

yonghong-song added a comment.

Hi, @aeubanks , this patch is hurting bpf backend.
The bpf backend calling convention is only allowing 5
parameters in registers and does not allow pass arguments
through stacks. But ArgumentPromotionPass could increase
the number of arguments for the function. If the eventual
number of arguments is more than 5, bpf backend will
have a fatal error.

The issue is discovered when I am compiling with latest clang
on bpf selftests, By default, for bpf community, it is recommended
to use -O2 as the compilation flag and that is why the issue is
not exposed earlier. This patch enabled the transformation at -O2,
and I hit the following errors with latest clang:

  ...
  progs/test_xdp_noinline.c:739:8: error: too many args to t8: i64 = GlobalAddress<ptr @encap_v4> 0, progs/test_xdp_noinline.c:739:8
                if (!encap_v4(xdp, cval, &pckt, dst, pkt_bytes))
                     ^
  ...
  progs/test_xdp_noinline.c:321:6: error: defined with too many args
  bool encap_v4(struct xdp_md *xdp, struct ctl_value *cval,
       ^
  ...

The following is a further analysis why the above happens.

  struct flow_key {
        union {
                __be32 src;
                __be32 srcv6[4];
        };
        union {
                __be32 dst;
                __be32 dstv6[4];
        };
        union {
                __u32 ports;
                __u16 port16[2];
        };
        __u8 proto;
  };
  struct packet_description {
        struct flow_key flow;
        __u8 flags;
  };
  static __attribute__ ((noinline))
  bool encap_v4(struct xdp_md *xdp, struct ctl_value *cval,
                struct packet_description *pckt,
                struct real_definition *dst, __u32 pkt_bytes)
  {
    ...
    __u32 ip_suffix = bpf_ntohs(pckt->flow.port16[0]);
    ...
    ip_suffix ^= pckt->flow.src;
    ...
  }
  __attribute__ ((noinline))
  static int process_packet(...) {
    struct packet_description pckt = { };
    ...
    encap_v4(xdp, cval, &pckt, dst, pkt_bytes)
    ...
  }

In function encap_v4, for argument pckt, only pckt->flow_port16[0]
and pckt->flow_src are referenced.

Before ArgumentPromotionPass pass, encap_v4 signature (in IR):

  i1 @encap_v4(ptr noundef %xdp, ptr noundef %cval, ptr noundef %pckt, ptr noundef %dst, i32 noundef %pkt_bytes)

After ArgumentPromotionPass pass, encap_v4 signature (in IR):

  i1 @encap_v4(ptr noundef %xdp, ptr noundef %cval, i32 %pckt.0.val, i16 %pckt.32.val, ptr noundef %dst, i32 noundef %pkt_bytes)

Basically, the argument 'pckt' is replaced with two arguments corresponding two pckt dereferences
inside encap_v4. But this transformation increases the number of arguments to 6 and eventually
bpf backend complains and errors out.

What is the proper way to resolve this? Adding a TTI hook to get target maximum number of
arguments? E.g, if -1, means no constraints, otherwise, it should return a >= 0 number which
indicates the maximum number of allowed arguments? Is it possible ArgumentPromotionPass may
replace the old reference parameter with a new struct argument with struct size > 8? If this
is the case we will have problem too.

Or for simplicity, simply disable this transformation for bpf backend?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148269/new/

https://reviews.llvm.org/D148269