[PATCH] D104542: [M68k][GloballSel] Formal arguments lowering in IRTranslator

[Min-Yih Hsu via Phabricator via llvm-commits]

myhsu added a comment.

In D104542#2842737 <https://reviews.llvm.org/D104542#2842737>, @sushmaunnibhavi wrote:

> In D104542#2842715 <https://reviews.llvm.org/D104542#2842715>, @myhsu wrote:
>
>> In D104542#2842366 <https://reviews.llvm.org/D104542#2842366>, @sushmaunnibhavi wrote:
>>
>>> In D104542#2840195 <https://reviews.llvm.org/D104542#2840195>, @myhsu wrote:
>>>
>>>> I'm curious if the test pass on your side. When I applied this patch on tip-of-tree, the newly-added test case triggered unreachable statement on line 87 of `lib/Target/M68k/GlSel/M68kCallLowering.cpp` (i.e. `M68kIncomingValueHandler::getStackAddress` was called).
>>>> Can you double check if this patch works on tip-of-tree? (of course please make sure you have assertion enabled in your build)
>>>
>>> I did build my patch again but I don't get any errors.
>>
>> no, I didn't say it fail the build, I said it failed the test after this patch was applied.
>> In other words, `ninja check-llvm-codegen-m68k` fails (or replace `ninja` with whatever build system you're using). Can you verify whether that command fails on your side?
>
> Yes I did this too and the tests didn't fail.
>
> F17618547: Screenshot from 2021-06-27 10-09-23.png <https://reviews.llvm.org/F17618547>

Hmmm...alright, let's see how the buildbot will say



================
Comment at: llvm/lib/Target/M68k/GlSel/M68kCallLowering.h:68
+
+public:
+  FormalArgHandler(MachineIRBuilder &MIRBuilder, MachineRegisterInfo &MRI)
----------------
myhsu wrote:
> please use struct instead and remove "public" in this line as well as the empty line above
still outstanding


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

https://reviews.llvm.org/D104542