[PATCH] D120545: [LoongArch] Add EncoderMethods for transformed immediate operands

[Lu Weining via Phabricator via llvm-commits]

SixWeining added inline comments.


================
Comment at: llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchMCCodeEmitter.cpp:97
+
+  llvm_unreachable("Unhandled expression!");
+}
----------------
rengolin wrote:
> Not sure what's better here, `assert(MO.isImm())` or `llvm_unreachable`.
> 
> On the function above, you return 0 even after llvm_unreachable, but here you don't.
> 
> Is that because of compiler warnings? I believe this shouldn't be necessary, but I could be wrong.
Seems `assert` is better because this function is dedicated for the  `uimm2_plus1` immideate operand. It's impossible to be other operand.

As @MaskRay said, `llvm_unreachable` does not need a following return. In `llvm/include/llvm/Support/ErrorHandling.h`, I see:

```
/// This function calls abort(), and prints the optional message to stderr.
/// Use the llvm_unreachable macro (that adds location info), instead of
/// calling this function directly.
[[noreturn]] void
llvm_unreachable_internal(const char *msg = nullptr, const char *file = nullptr,
                          unsigned line = 0); 
}

/// Marks that the current location is not supposed to be reachable.
/// In !NDEBUG builds, prints the message and location info to stderr.
/// In NDEBUG builds, becomes an optimizer hint that the current location
/// is not supposed to be reachable.  On compilers that don't support
/// such hints, prints a reduced message instead and aborts the program.
///
/// Use this instead of assert(0).  It conveys intent more clearly and
/// allows compilers to omit some unnecessary code.
#ifndef NDEBUG
#define llvm_unreachable(msg) \
  ::llvm::llvm_unreachable_internal(msg, __FILE__, __LINE__)
#elif defined(LLVM_BUILTIN_UNREACHABLE)
#define llvm_unreachable(msg) LLVM_BUILTIN_UNREACHABLE
#else
#define llvm_unreachable(msg) ::llvm::llvm_unreachable_internal()
#endif

#endif
```

Here `llvm_unreachable_internal()` is marked with `[[noreturn]]`.

As the above function, since it's not introduced in this patch, I will delete the unnecessary `return 0` in a seperate patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120545