[llvm] [SPIRV] Start adding support for `int128` (PR #170798)

[Alex Voicu via llvm-commits]

AlexVlx wrote:

> > Hi @AlexVlx
> > I noticed that if I build with gcc and -DLLVM_ENABLE_EXPENSIVE_CHECKS=ON many SPIRV lit tests fail with this patch.
> > E.g. `build-all-gcc132-expensive/bin/llc -O0 -verify-machineinstrs -mtriple=spirv64-unknown-unknown --spirv-ext=+SPV_ALTERA_arbitrary_precision_integers,+SPV_KHR_bit_instructions test/CodeGen/SPIRV/llvm-intrinsics/bitreverse_small_type.ll -o -` fails with
> > ```
> > [...]/gcc/13.2.0/include/c++/13.2.0/bits/stl_algo.h:4892:
> > In function:
> >     void std::sort(_RAIter, _RAIter, _Compare) [with _RAIter =
> >     llvm::StringRef*; _Compare =
> >     llvm::SPIRVExtensionsParser::parse(llvm::cl::Option&, llvm::StringRef,
> >     llvm::StringRef, std::
> >     debug::set<llvm::SPIRV::Extension::Extension>&)::<lambda(auto:4&&,
> >     auto:5&&)>]
> > 
> > Error: comparison doesn't meet irreflexive requirements, assert(!(a < a)).
> > 
> > Objects involved in the operation:
> >     instance "functor" @ 0x7ffd92e1a6a0 {
> >     }
> >     iterator::value_type "ordered type"  {
> >     }
> > PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace and instructions to reproduce the bug.
> > Stack dump:
> > 0.      Program arguments: build-all-gcc132-expensive/bin/llc -O0 -verify-machineinstrs -mtriple=spirv64-unknown-unknown --spirv-ext=+SPV_ALTERA_arbitrary_precision_integers,+SPV_KHR_bit_instructions test/CodeGen/SPIRV/llvm-intrinsics/bitreverse_small_type.ll -o -
> >  #0 0x0000000004e8fc6e llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (build-all-gcc132-expensive/bin/llc+0x4e8fc6e)
> >  #1 0x0000000004e8c5cb llvm::sys::RunSignalHandlers() (build-all-gcc132-expensive/bin/llc+0x4e8c5cb)
> >  #2 0x0000000004e8c70e SignalHandler(int, siginfo_t*, void*) Signals.cpp:0:0
> >  #3 0x00007fa6f7f28990 __restore_rt (/lib64/libpthread.so.0+0x12990)
> >  #4 0x00007fa6f58bf52f raise (/lib64/libc.so.6+0x4e52f)
> >  #5 0x00007fa6f5892e65 abort (/lib64/libc.so.6+0x21e65)
> >  #6 0x000000000080410d (build-all-gcc132-expensive/bin/llc+0x80410d)
> >  #7 0x000000000246ef29 llvm::SPIRVExtensionsParser::parse(llvm::cl::Option&, llvm::StringRef, llvm::StringRef, std::__debug::set<llvm::SPIRV::Extension::Extension, std::less<llvm::SPIRV::Extension::Extension>, std::allocator<llvm::SPIRV::Extension::Extension>>&) crtstuff.c:0:0
> >  #8 0x00000000024257e5 llvm::cl::opt<std::__debug::set<llvm::SPIRV::Extension::Extension, std::less<llvm::SPIRV::Extension::Extension>, std::allocator<llvm::SPIRV::Extension::Extension>>, false, llvm::SPIRVExtensionsParser>::handleOccurrence(unsigned int, llvm::StringRef, llvm::StringRef) crtstuff.c:0:0
> >  #9 0x0000000004d4b5c1 llvm::cl::ParseCommandLineOptions(int, char const* const*, llvm::StringRef, llvm::raw_ostream*, llvm::vfs::FileSystem*, char const*, bool) (build-all-gcc132-expensive/bin/llc+0x4d4b5c1)
> > #10 0x0000000000806968 main (build-all-gcc132-expensive/bin/llc+0x806968)
> > #11 0x00007fa6f58ab7e5 __libc_start_main (/lib64/libc.so.6+0x3a7e5)
> > #12 0x00000000009510ee _start (build-all-gcc132-expensive/bin/llc+0x9510ee)
> > Abort (core dumped)
> > ```
> > 
> > 
> >     
> >       
> >     
> > 
> >       
> >     
> > 
> >     
> >   
> > I guess it's this change that cause this
> > ```
> >  bool SPIRVExtensionsParser::parse(cl::Option &O, StringRef ArgName,
> >                                    StringRef ArgValue,
> >                                    std::set<SPIRV::Extension::Extension> &Vals) {
> >    SmallVector<StringRef, 10> Tokens;
> >    ArgValue.split(Tokens, ",", -1, false);
> > -  std::sort(Tokens.begin(), Tokens.end());
> > +  llvm::sort(Tokens, [](auto &&LHS, auto &&RHS) {
> > +    // We want to ensure that we handle "all" first, to ensure that any
> > +    // subsequent disablement actually behaves as expected i.e. given
> > +    // --spv-ext=all,-foo, we first enable all and then disable foo; this should
> > +    // be revisited and simplified.
> > +    if (LHS == "all")
> > +      return true;
> > +    if (RHS == "all")
> > +      return false;
> > +    return !(RHS < LHS);
> > +  });
> > ```
> 
> @AlexVlx https://en.cppreference.com/w/cpp/algorithm/sort.html says the following:
> 
> ```
> comp 	- 	comparison function object (i.e. an object that satisfies the requirements of [Compare](https://en.cppreference.com/w/cpp/named_req/Compare.html)) which returns ​true if the first argument is less than (i.e. is ordered before) the second.
> ```
> 
> and then https://en.cppreference.com/w/cpp/named_req/Compare.html has the following requirement
> 
> ```
>  For all a, comp(a, a) == false. 
> ```
> 
> And with this patch we instead do
> 
> ```
> +    if (LHS == "all")
> +      return true;
> +    if (RHS == "all")
> +      return false;
> ```
> 
> so `comp("all","all") == true` which doesn't follow the requirement.

Thank you for the analysis. #171826 addresses this.

https://github.com/llvm/llvm-project/pull/170798