[PATCH] Disable DAGCombine for -O0 and optnone

Demikhovsky, Elena elena.demikhovsky at intel.com
Wed Jun 3 06:50:56 PDT 2015


I just afraid that some tests will fail to select instruction if you run them without DAGCombine.
So you should run ALL tests with O0 and be sure that they don't fail.

-  Elena

-----Original Message-----
From: Oleg Ranevskyy [mailto:llvm.mail.list at gmail.com] 
Sent: Wednesday, June 03, 2015 16:46
To: llvm.mail.list at gmail.com; echristo at gmail.com; grosbach at apple.com; Paul_Robinson at playstation.sony.com; Demikhovsky, Elena
Cc: compnerd at compnerd.org; amara.emerson at arm.com; Kuperstein, Michael M; anton at korobeynikov.info; llvm-commits at cs.uiuc.edu
Subject: Re: [PATCH] Disable DAGCombine for -O0 and optnone

In http://reviews.llvm.org/D9992#182779, @delena wrote:

> All lit tests should pass in the -O0 mode. The code may be not optimal, but the compilation should not fail and the generated code should correct.


Not sure I get this point correctly.
Many tests currently run without -O0 and their CHECK's expect some optimizations to happen. This doesn't mean the tests won't compile with -O0. This means the CHECK's will have to be changed if -O0 is used.

The patch has disabled DAGCombine for -O0. This broke some tests running with -O0 as their CHECK's were written with DAGCombine in mind. I have changed the CHECK's for those tests whose differences in the code generated with and without DAGCombine in -O0 were relatively small. Several tests however had quite significant differences in the generated code, and required full redesign of their CHECK's. Instead, DAGCombine was enabled for them by changing -O0 -> -O1 to get nearly the same code as the CHECK's expected. Please let me know if this is not the best way to handle such tests and we should rather stick to the 1st option (leave -O0 and redesign the CHECK's).


REPOSITORY
  rL LLVM

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:17616
@@ -17608,1 +17615,3 @@
 
+SDValue X86TargetLowering::LowerFMA(SDValue Op, SelectionDAG &DAG) const {
+  SDLoc dl(Op);
----------------
delena wrote:
> You should not do the work of DAG-combiner in lowering. Lowering should deal with one SDNode.
> Please put the FMA-combine back and just translate ISD::FMA to X86ISD::FMADD here.
Thank you for the clarification Elena. I will submit a new patch shortly.

http://reviews.llvm.org/D9992

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/


---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.




More information about the llvm-commits mailing list