Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement pass to do TanhOp lowering. #10

Merged
merged 1 commit into from
Jun 9, 2020

Conversation

LLITCHEV
Copy link

@LLITCHEV LLITCHEV commented Jun 9, 2020

This is the change that I'll submit to upstream. That is what they wanted - create a pass to do the lowering before the StdxToLLVm pass. Then changes to the LLVM pass are not needed.
I'll commit this here, so I can commit a corresponding PR on the plaidml-v1 and when this lands to upstream I'll remove it.

@LLITCHEV LLITCHEV merged commit e41f9b1 into plaidml/plaidml-v1 Jun 9, 2020
@flaub flaub deleted the llitchev-tanh-impl-ex2 branch June 9, 2020 19:09
haoyouab pushed a commit to Flex-plaidml-team/llvm-project that referenced this pull request Sep 4, 2020
When `Target::GetEntryPointAddress()` calls `exe_module->GetObjectFile()->GetEntryPointAddress()`, and the returned
`entry_addr` is valid, it can immediately be returned.

However, just before that, an `llvm::Error` value has been setup, but in this case it is not consumed before returning, like is done further below in the function.

In https://bugs.freebsd.org/248745 we got a bug report for this, where a very simple test case aborts and dumps core:

```
* thread plaidml#1, name = 'testcase', stop reason = breakpoint 1.1
    frame #0: 0x00000000002018d4 testcase`main(argc=1, argv=0x00007fffffffea18) at testcase.c:3:5
   1	int main(int argc, char *argv[])
   2	{
-> 3	    return 0;
   4	}
(lldb) p argc
Program aborted due to an unhandled Error:
Error value was Success. (Note: Success values must still be checked prior to being destroyed).

Thread 1 received signal SIGABRT, Aborted.
thr_kill () at thr_kill.S:3
3	thr_kill.S: No such file or directory.
(gdb) bt
#0  thr_kill () at thr_kill.S:3
plaidml#1  0x00000008049a0004 in __raise (s=6) at /usr/src/lib/libc/gen/raise.c:52
plaidml#2  0x0000000804916229 in abort () at /usr/src/lib/libc/stdlib/abort.c:67
plaidml#3  0x000000000451b5f5 in fatalUncheckedError () at /usr/src/contrib/llvm-project/llvm/lib/Support/Error.cpp:112
plaidml#4  0x00000000019cf008 in GetEntryPointAddress () at /usr/src/contrib/llvm-project/llvm/include/llvm/Support/Error.h:267
plaidml#5  0x0000000001bccbd8 in ConstructorSetup () at /usr/src/contrib/llvm-project/lldb/source/Target/ThreadPlanCallFunction.cpp:67
plaidml#6  0x0000000001bcd2c0 in ThreadPlanCallFunction () at /usr/src/contrib/llvm-project/lldb/source/Target/ThreadPlanCallFunction.cpp:114
plaidml#7  0x00000000020076d4 in InferiorCallMmap () at /usr/src/contrib/llvm-project/lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp:97
plaidml#8  0x0000000001f4be33 in DoAllocateMemory () at /usr/src/contrib/llvm-project/lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp:604
plaidml#9  0x0000000001fe51b9 in AllocatePage () at /usr/src/contrib/llvm-project/lldb/source/Target/Memory.cpp:347
plaidml#10 0x0000000001fe5385 in AllocateMemory () at /usr/src/contrib/llvm-project/lldb/source/Target/Memory.cpp:383
plaidml#11 0x0000000001974da2 in AllocateMemory () at /usr/src/contrib/llvm-project/lldb/source/Target/Process.cpp:2301
plaidml#12 CanJIT () at /usr/src/contrib/llvm-project/lldb/source/Target/Process.cpp:2331
plaidml#13 0x0000000001a1bf3d in Evaluate () at /usr/src/contrib/llvm-project/lldb/source/Expression/UserExpression.cpp:190
plaidml#14 0x00000000019ce7a2 in EvaluateExpression () at /usr/src/contrib/llvm-project/lldb/source/Target/Target.cpp:2372
plaidml#15 0x0000000001ad784c in EvaluateExpression () at /usr/src/contrib/llvm-project/lldb/source/Commands/CommandObjectExpression.cpp:414
plaidml#16 0x0000000001ad86ae in DoExecute () at /usr/src/contrib/llvm-project/lldb/source/Commands/CommandObjectExpression.cpp:646
plaidml#17 0x0000000001a5e3ed in Execute () at /usr/src/contrib/llvm-project/lldb/source/Interpreter/CommandObject.cpp:1003
plaidml#18 0x0000000001a6c4a3 in HandleCommand () at /usr/src/contrib/llvm-project/lldb/source/Interpreter/CommandInterpreter.cpp:1762
plaidml#19 0x0000000001a6f98c in IOHandlerInputComplete () at /usr/src/contrib/llvm-project/lldb/source/Interpreter/CommandInterpreter.cpp:2760
plaidml#20 0x0000000001a90b08 in Run () at /usr/src/contrib/llvm-project/lldb/source/Core/IOHandler.cpp:548
plaidml#21 0x00000000019a6c6a in ExecuteIOHandlers () at /usr/src/contrib/llvm-project/lldb/source/Core/Debugger.cpp:903
plaidml#22 0x0000000001a70337 in RunCommandInterpreter () at /usr/src/contrib/llvm-project/lldb/source/Interpreter/CommandInterpreter.cpp:2946
plaidml#23 0x0000000001d9d812 in RunCommandInterpreter () at /usr/src/contrib/llvm-project/lldb/source/API/SBDebugger.cpp:1169
plaidml#24 0x0000000001918be8 in MainLoop () at /usr/src/contrib/llvm-project/lldb/tools/driver/Driver.cpp:675
plaidml#25 0x000000000191a114 in main () at /usr/src/contrib/llvm-project/lldb/tools/driver/Driver.cpp:890```

Fix the incorrect error catch by only instantiating an `Error` object if it is necessary.

Reviewed By: JDevlieghere

Differential Revision: https://reviews.llvm.org/D86355
chelini pushed a commit that referenced this pull request Sep 25, 2022
Commit de3445e (https://reviews.llvm.org/D132096) made
changes to isVectorPromotionViable basically doing

  // Create Vector with size of V, and each element of type Ty
  ...
  uint64_t ElementSize = DL.getTypeStoreSizeInBits(Ty).getFixedSize();
  uint64_t VectorSize = DL.getTypeSizeInBits(V).getFixedSize();
  ...
  VectorType *VTy = VectorType::get(Ty, VectorSize / ElementSize, false);

Not quite sure why it uses the TypeStoreSize for the ElementSize,
but the new vector would only match in size with the old vector in
situations when the TypeStoreSize equals the TypeSize for Ty.
Therefore this patch adds a typeSizeEqualsStoreSize check as yet
another condition for allowing the the new type as a promotion
candidate.

Without this fix the new @test15 test would fail with an assert
like this:

opt: ../lib/Transforms/Scalar/SROA.cpp:1966:
  auto isVectorPromotionViable(llvm::sroa::Partition &,
                               const llvm::DataLayout &)
       ::(anonymous class)::operator()(llvm::VectorType *,
                                       llvm::VectorType *) const:
  Assertion `DL.getTypeSizeInBits(RHSTy).getFixedSize() ==
             DL.getTypeSizeInBits(LHSTy).getFixedSize() &&
             "Cannot have vector types of different sizes!"' failed.
 ...
 #8  isVectorPromotionViable(...)::$_10::operator()...
 #9  llvm::SROAPass::rewritePartition(...)
#10  llvm::SROAPass::splitAlloca(...)
#11  llvm::SROAPass::runOnAlloca(...)
#12  llvm::SROAPass::runImpl(...)
#13  llvm::SROAPass::run(...)

Reviewed By: MatzeB

Differential Revision: https://reviews.llvm.org/D134032
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants