[PATCH] D156497: [BPF] report fatal error rather than miscompile

Wed Aug 2 18:34:36 PDT 2023

ast added a comment.

In D156497#4556048 <https://reviews.llvm.org/D156497#4556048>, @tamird wrote:

> In D156497#4555836 <https://reviews.llvm.org/D156497#4555836>, @ast wrote:
>
>> Agree with Eduard. Doing report_fatal_error() in fail() is cleaner. Proceeding with miscompiled code can only cause further issues in other parts of the backend.
>> Not sure why we didn't do this earlier.
>
> Counterpoint: D98471 <https://reviews.llvm.org/D98471>. @yonghong-song can you weigh in please?
>
> It's not apparent that there's an obviously right thing to do here. The standard error emitting mechanisms seem to always involve a risk of miscompilation. Does anyone know how other backends deal with this situation, where code generation cannot reasonably proceed? Looks like other backends use `UNDEF` in at least some similar cases (https://github.com/llvm/llvm-project/blob/cd328c10ad503b243cc333f813c0c6ee66598ffc/llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp#L1265-L1269). From my perspective, as long as we miscompile more severely - that is we produce code that definitely doesn't work, rather than code that might work but do the wrong thing - then I'm happy. @ast I believe you suggested constant zeroes rather than UNDEF in earlier reviews -- can you shed some light on why that was deemed preferable?

actually. let's step back.
The compilation doesn't report success at the end, so what is the problem?
diagnose() and proceed is a normal pattern used in many backends.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156497