[PATCH] D156136: [BPF] Clean up SelLowering

[Eduard Zingerman via Phabricator via llvm-commits]

eddyz87 added inline comments.


================
Comment at: llvm/lib/Target/BPF/BPFISelLowering.cpp:46
+    Val->print(OS);
+    OS << ' ';
+  }
----------------
tamird wrote:
> eddyz87 wrote:
> > This prints extra space at the end of the output. I think it should be like this:
> > 
> > ```
> >   if (Val) {
> >     raw_string_ostream OS(Str);
> >     OS << ' ';
> >     Val->print(OS);
> >   }
> > ```
> I don't think so; the caller has passed in a message, which we concat on line 50. We need this space so that the user's message doesn't blend in with ours.
I mean, I checked it before commenting.
If you add, say `'|'`, at the beginning and at the end of `OS` you'll get output like this:

```
test3.c:3:10: error: too many arguments|t1: i64 = GlobalAddress<ptr @foo> 0, test3.c:3:10 |
    3 |   return foo(1, 2, 3, 4, 5, 6, 7);
      |          ^
1 error generated.
```

Note that there is no space after "arguments" and there is an extra space before last `|`. This is because we are doing `Msg.concat(Str)`, not `Str.concat(Msg)`.

Thanks for addressing the rest of the comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156136