[llvm-commits] [PATCH][MCJIT] Removing DefaultJITMemoryManager from MCJITMemoryManager

[Kaylor, Andrew]

The way I was looking at it, if the client uses a NULL JMM in the current implementation, it might work (or it might fail), whereas with your change it will definitely fail.

I can understand why you would consider the definite failure to be preferable, but the circumstances under which the current implementation works (relatively small modules) are such that many clients may never see the failure, and it would be a shame to break existing clients when we don't need to.

That said, I don't have extremely strong feelings about this and am willing to live with it either way.

-Andy

From: Danil Malyshev [mailto:dmalyshev at accesssoftek.com]
Sent: Thursday, July 26, 2012 3:50 PM
To: Kaylor, Andrew; llvm-commits at cs.uiuc.edu
Subject: RE: [llvm-commits] [PATCH][MCJIT] Removing DefaultJITMemoryManager from MCJITMemoryManager

Hi Andrew


I agree that the absence of default does not looks good.
But in my view, using a non-working MemoryManager as default is even worse, because misleading.
I think that LLIMCJITMemoryManager a good candidate for the default value, but I do not see as it's possible to use it for that right now. When the JIT goes out, the DefaultJITMemoryManager can be merged with LLIMCJITMemoryManager and then it can be used as default MM again.



Regards,
Danil

________________________________
From: Kaylor, Andrew [mailto:andrew.kaylor at intel.com]
Sent: Thursday, July 26, 2012 10:29 PM
To: Danil Malyshev; llvm-commits at cs.uiuc.edu<mailto:llvm-commits at cs.uiuc.edu>
Subject: RE: [llvm-commits] [PATCH][MCJIT] Removing DefaultJITMemoryManager from MCJITMemoryManager

It seems like a bad idea to start requiring a non-NULL JMM.  It wasn't long ago that the reference implementations like lli were leaving that NULL, and I believe that for small objects it works.  That said, I do think something needs to be done to get a better default behavior for MCJIT.

Did you happen to see my recent proposal for MCJIT enhancements?

-Andy

From: llvm-commits-bounces at cs.uiuc.edu<mailto:llvm-commits-bounces at cs.uiuc.edu> [mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Danil Malyshev
Sent: Wednesday, July 25, 2012 5:18 PM
To: llvm-commits at cs.uiuc.edu<mailto:llvm-commits at cs.uiuc.edu>
Subject: [llvm-commits] [PATCH][MCJIT] Removing DefaultJITMemoryManager from MCJITMemoryManager

Hi everyone,


Please review attached the patch.
The MCJITMemoryManager shouldn't use the DefaultJITMemoryManager by following reason:
1. It does not have any methods for invalidating instruction cache, so attempt to execute the code might be fail in ARM platforms.
2. The repeated call of DefaultJITMemoryManager.allocateDataSection() fails.
But the MCJITMemoryManager misleading because his constructor create the DefaultJITMemoryManager if jmm parameter is empty. This patch fixed it.


Regards,
Danil
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120726/cf87aac5/attachment.html>