[PATCH] D156136: [BPF] Clean up SelLowering
Eduard Zingerman via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 28 13:29:37 PDT 2023
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
More information about the llvm-commits
mailing list